Primitive Python P2P Socket Chat

Question

Here is my code for a Python script for a peer to peer chat system on a network using sockets. I know there are some things that could be changed so there would be less repetition in the code and that some variables could be global, but what I am asking is if there are any issues with the code's functionality; it seems to work properly on my home network but will only connect (not receive messages) on my school network. Because it establishes a connection there, it looks like the port is not being blocked by the internal firewall.

#!usr/bin/env python

import socket
import threading
import select
import time
import datetime

def main():

    class Chat_Server(threading.Thread):
            def __init__(self):
                threading.Thread.__init__(self)
                self.running = 1
                self.conn = None
                self.addr = None
            def run(self):
                HOST = ''
                PORT = 8080
                s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
                s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
                s.bind((HOST,PORT))
                s.listen(1)
                self.conn, self.addr = s.accept()
                # Select loop for listen
                while self.running == True:
                    inputready,outputready,exceptready \
                      = select.select ([self.conn],[self.conn],[])
                    for input_item in inputready:
                        # Handle sockets
                        message = self.conn.recv(1024)
                        if message:
                            print "Daniel: " + message + ' (' + datetime.datetime.now().strftime('%H:%M:%S') + ')'
                        else:
                            break
                    time.sleep(0)
            def kill(self):
                self.running = 0

    class Chat_Client(threading.Thread):
            def __init__(self):
                threading.Thread.__init__(self)
                self.host = None
                self.sock = None
                self.running = 1
            def run(self):
                PORT = 8080
                self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
                self.sock.connect((self.host, PORT))
                # Select loop for listen
                while self.running == True:
                    inputready,outputready,exceptready \
                      = select.select ([self.sock],[self.sock],[])
                    for input_item in inputready:
                        # Handle sockets
                        message = self.sock.recv(1024)
                        if message:
                            print "Daniel: " + message + ' (' + datetime.datetime.now().strftime('%H:%M:%S') + ')'
                        else:
                            break
                    time.sleep(0)
            def kill(self):
                self.running = 0

    class Text_Input(threading.Thread):
            def __init__(self):
                threading.Thread.__init__(self)
                self.running = 1
            def run(self):
                while self.running == True:
                  text = raw_input('')
                  try:
                      chat_client.sock.sendall(text)
                  except:
                      Exception
                  try:
                      chat_server.conn.sendall(text)
                  except:
                      Exception
                  time.sleep(0)
            def kill(self):
                self.running = 0

    # Prompt, object instantiation, and threads start here.

    ip_addr = raw_input('Type IP address or press enter: ')

    if ip_addr == '':
        chat_server = Chat_Server()
        chat_client = Chat_Client()
        chat_server.start()
        text_input = Text_Input()
        text_input.start()

    else:
        chat_server = Chat_Server()
        chat_client = Chat_Client()
        chat_client.host = ip_addr
        text_input = Text_Input()
        chat_client.start()
        text_input.start()

if __name__ == "__main__":
    main()

Answer 1

Regular review.

• main is bloated. All the class definitions do net belong there.

• Creating a Chat_Client instance in a server branch (and vice versa) looks strange. My guess it is a result of a Text_Input design, which assumes that both server and client exist. This is of course wrong. A Text_Input instance should have just one "sink" where the text input would go:

  class Text_Input(threading.Thread):
          def __init__(self, sink):
              threading.Thread.__init__(self)
              self.sink = sink
              self.running = 1
          def run(self):
              while self.running == True:
                text = raw_input('')
                self.sink.send(text)
  

  and server initialization becomes

  chat_server = Chat_Server()
  text_input = TextInput(chat_server)
  

  Same goes for a client. Both server and client should implement send method.

• Network initialization should be moved into constructors. After that, run and send method (same for client and server) should be factored out into a separate Chatter class which client and server inherit from.

• What is the purpose of time.sleep(0)? If the system misbehaves without it, then there is a bug to be fixed, not masked. Hint: you should select for outputready only when there is a message to be sent.

Answer 2

Try using a really weird port number as port 8080 is commonly used for a proxy port.

Also, the client doesn't make any effort to verify it indeed connects to the server you want it to. It could very easily have connected to something else that is already listening on 8080, and it will sit there doing nothing basically forever.

You really should .shutdown() and .close() the sockets before you close the clients in order to prevent handle leaks.

I would recommend you come up with a novel SYN SYN-ACK ACK using a unique signature so that if the client connects to something other than the valid server it will close the socket, notify the user, and close. And so that if an invalid Client connects to the server, The server can Disconnect the invalid session, notify the user, and continue waiting for a valid client to connect.