views:

72

answers:

1

This is my 'game server'. It's nothing serious, I thought this was a nice way of learning a few things about python and sockets.

First the server class initialized the server. Then, when someone connects, we create a client thread. In this thread we continually listen on our socket.

Once a certain command comes in (I12345001001, for example) it spawns another thread.

The purpose of this last thread is to send updates to the client. But even though I see the server is performing this code, the data isn't actually being sent.

Could anyone tell where it's going wrong? It's like I have to receive something before I'm able to send. So I guess somewhere I'm missing something


#!/usr/bin/env python

"""
An echo server that uses threads to handle multiple clients at a time.
Entering any line of input at the terminal will exit the server.
"""

import select
import socket
import sys
import threading
import time
import Queue

globuser = {}
queue = Queue.Queue()

class Server:
    def __init__(self):
        self.host = ''
        self.port = 2000
        self.backlog = 5
        self.size = 1024
        self.server = None
        self.threads = []

    def open_socket(self):
        try:
            self.server = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
            self.server.bind((self.host,self.port))
            self.server.listen(5)
        except socket.error, (value,message):
            if self.server:
                self.server.close()
            print "Could not open socket: " + message
            sys.exit(1)

    def run(self):
        self.open_socket()
        input = [self.server,sys.stdin]
        running = 1
        while running:
            inputready,outputready,exceptready = select.select(input,[],[])

            for s in inputready:

                if s == self.server:
                    # handle the server socket
                    c = Client(self.server.accept(), queue)
                    c.start()
                    self.threads.append(c)

                elif s == sys.stdin:
                    # handle standard input
                    junk = sys.stdin.readline()
                    running = 0

        # close all threads

        self.server.close()
        for c in self.threads:
            c.join()

class Client(threading.Thread):
    initialized=0

    def __init__(self,(client,address), queue):
        threading.Thread.__init__(self)
        self.client = client
        self.address = address
        self.size = 1024
        self.queue = queue
        print 'Client thread created!'


    def run(self):
        running = 10
        isdata2=0
        receivedonce=0

        while running > 0:

            if receivedonce == 0:
                print 'Wait for initialisation message'
                data = self.client.recv(self.size)
                receivedonce = 1

            if self.queue.empty():
                print 'Queue is empty'
            else:
                print 'Queue has information'
                data2 = self.queue.get(1, 1)
                isdata2 = 1
                if data2 == 'Exit':
                    running = 0
                    print 'Client is being closed'
                    self.client.close()

            if data:
                print 'Data received through socket! First char: "' + data[0] + '"'
                if data[0] == 'I':
                    print 'Initializing user'
                    user = {'uid': data[1:6] ,'x': data[6:9], 'y': data[9:12]}
                    globuser[user['uid']] = user
                    print globuser
                    initialized=1
                    self.client.send('Beginning - Initialized'+';')
                    m=updateClient(user['uid'], queue)
                    m.start()
                else:
                    print 'Reset receivedonce'
                    receivedonce = 0

                print 'Sending client data'
                self.client.send('Feedback: ' +data+';')
                print 'Client Data sent: ' + data

            data=None

            if isdata2 == 1:
                print 'Data2 received: ' + data2
                self.client.sendall(data2)
                self.queue.task_done()
                isdata2 = 0

            time.sleep(1)
            running = running - 1

        print 'Client has stopped'


class updateClient(threading.Thread):

    def __init__(self,uid, queue):
        threading.Thread.__init__(self)
        self.uid = uid
        self.queue = queue
        global globuser
        print 'updateClient thread started!'

    def run(self):
        running = 20
        test=0
        while running > 0:
            test = test + 1
            self.queue.put('Test Queue Data #' + str(test))
            running = running - 1
            time.sleep(1)

        print 'Updateclient has stopped'


if __name__ == "__main__":
    s = Server()
    s.run() 

+2  A: 

I don't understand your logic -- in particular, why you deliberately set up two threads writing at the same time on the same socket (which they both call self.client), without any synchronization or coordination, an arrangement that seems guaranteed to cause problems.

Anyway, a definite bug in your code is you use of the send method -- you appear to believe that it guarantees to send all of its argument string, but that's very definitely not the case, see the docs:

Returns the number of bytes sent. Applications are responsible for checking that all data has been sent; if only some of the data was transmitted, the application needs to attempt delivery of the remaining data.

sendall is the method that you probably want:

Unlike send(), this method continues to send data from string until either all data has been sent or an error occurs.

Other problems include the fact that updateClient is apparently designed to never terminate (differently from the other two thread classes -- when those terminate, updateClient instances won't, and they'll just keep running and keep the process alive), redundant global statements (innocuous, just confusing), some threads trying to read a dict (via the iteritems method) while other threads are changing it, again without any locking or coordination, etc, etc -- I'm sure there may be even more bugs or problems, but, after spotting several, one's eyes tend to start to glaze over;-).

Alex Martelli
Hmm, I changed it to a sendall function but it still won't go.I actually made another question about the global dictionary that needs to be read and written to at the same time. Someone suggested __slots__, someone suggested multiprocessing. Nobody really talked about the bad things that could happen. I'll lookup the locking thing.And yes, it's 100% hackish :P
skerit
"read and written to at the same time" just won't fly -- use locks (or dedicated threads and intrinsically-threadsafe Queues as Jeremy suggests in a comment -- an even better architecture though it requires more reorg of your code).
Alex Martelli
I have successfully implemented a queue. Once you get the hang of it it's quite easy.But it still won't send the string to the socket, not even with sendall.So first it receives something from the socket. After that I can send something (although if I do 2 sends after one another, they're sent as one packet)Every other send is just ignored. Is it waiting to receive something again? Can't I flush it in some way?
skerit
Sockets don't do buffering and therefore have no flush methods (nor do they need one). If you edit your Q to show (instead of the super-buggy code that's there now) the code in which you think you have fixed the already-spotted bugs, I'll take a look and see what further bugs I can spot there. But working on that super-buggy code plus two comments from you is simply impossible.
Alex Martelli
@skerit - are you actually tracing execution of the code? I get the feeling that you are making assumptions about the execution sequence. My first guess based on your description of the current problem is that the Client thread blocks on `data = self.client.recv(self.size)` at the start of the loop, but you need to asynchronously send update messages. If that's the case, I suggest using `select` to poll the socket so that you are not blocked indefinitely from consuming the Queue... or you might find the `asyncore` module helpful
Jeremy Brown
I've edited a bit of the code, I mainly removed the global updating of the variable. I mainly want to be able to send from the socket without receiving anything.
skerit