views:

180

answers:

4

My class contains a socket that connects to a server. Some of the methods of the class can throw an exception. The script I'm running contains an outer loop that catches the exception, logs an error, and creates a new class instance that tries to reconnect to the server.

Problem is that the server only handles one connection at a time (by design) and the "old" socket is still connected. So the new connection attempt hangs the script. I can work around this by forcing the old socket closed, but I wonder: why doesn't the socket automatically close?

When it is "stuck", netstat shows two sockets connected to the port. The server is waiting for input from the first socket though, it isn't handling the new one yet.

I run this against a dummy server that replies "error\n" to every incoming line.

EDIT: see my comment on Mark Rushakoff's answer below. An assert(False) [that I subsequently catch] from within the exception handler seems to force the socket closed.

import socket

class MyException(Exception):
    pass

class MyClient(object):

    def __init__(self, port):
        self.sock = socket.create_connection(('localhost', port))
        self.sockfile = self.sock.makefile()

    def do_stuff(self):
        self._send("do_stuff\n")
        response = self._receive()
        if response != "ok\n":
            raise MyException()
        return response

    def _send(self, cmd):
        self.sockfile.write(cmd)
        self.sockfile.flush()

    def _receive(self):
        return self.sockfile.readline()

def connect():
    c = MyClient(9989)
    # On the second iteration, do_stuff() tries to send data and
    # hangs indefinitely.
    print c.do_stuff()

if __name__ == '__main__':
    for _ in xrange(3):
        try:
            connect()
        except MyException, e:
            print 'Caught:', e
            # This would be the workaround if I had access to the
            # MyClient object:
            #c.sock.close()
            #c.sockfile.close()

EDIT: Here's the (ugly) server code:

import socket
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM, 0)
s.bind(('localhost', 9989))
s.listen(5)
(c,a) = s.accept()
f = c.makefile()
print f.readline()
f.write('error\n')
f.flush()
(c2,a) = s.accept()
f = c.makefile()
print f.readline()
s.close()
+5  A: 

This is an artifact of garbage collection. Even though the object is out of scope, it is not necessarily collected and therefore destroyed until a garbage collection run occurs -- this is not like C++ where a destructor is called as soon as an object loses scope.

You can probably work around this particular issue by changing connect to

def connect():
    try:
        c = MyClient(9989)
        # On the second iteration, do_stuff() tries to send data and
        # hangs indefinitely.
        print c.do_stuff()
    finally:
        c.sock.close()
        c.sockfile.close()

Alternatively, you could define __enter__ and __exit__ for MyClient, and do a with statement:

def connect():
    with MyClient(9989) as c:
        print c.do_stuff()

Which is effectively the same as a try-finally.

Mark Rushakoff
I'm not sure this holds water. I just discovered that, if I wrap the try/except in another try/except AssertionError and assert(False) in the inner except block, then the socket gets cleaned up. I suspect it has something to do with the exception stack trace holding a reference to the object and thus to the socket?
bstpierre
Your example is not a complete program (server code not supplied), so it's somewhat difficult to confirm that behavior on this side.
Mark Rushakoff
Thanks for adding the workarounds. I don't have a problem working around it, I'm just trying to understand what's holding a ref to the object... and now I have to assume it's the traceback.
bstpierre
Edited to include the server.
bstpierre
Basically as Mark said you can't rely on an object being deallocated just because it moves out of scope. Just because garbage collection exists doesn't mean you should rely on it to trigger clean-up behaviours that are a side-effect of the object being garbage collected.
Benno
A: 

Ok, here's the final version. Explicitly close the socket objects when something gets borked.

import socket

class MyException(Exception):
    pass

class MyClient(object):

    def __init__(self, port):
        self.sock = socket.create_connection(('localhost', port))
        self.sockfile = self.sock.makefile()

    def close(self):
        self.sock.close()
        self.sockfile.close()

    def do_stuff(self):
        self._send("do_stuff\n")
        response = self._receive()
        if response != "ok\n":
            raise MyException()
        return response

    def _send(self, cmd):
        self.sockfile.write(cmd)
        self.sockfile.flush()

    def _receive(self):
        return self.sockfile.readline()

def connect():
    try:
        c = MyClient(9989)
        print c.do_stuff()
    except MyException:
        print 'Caught MyException'
    finally:
        c.close()

if __name__ == '__main__':
    for _ in xrange(2):
        connect()
bstpierre
I'm not sure why you're hesitant to just fix this now? The only reliable behaviour is to use 'finally' or the 'with' construct and moving your code around to get away from having to write the correct behaviour seems an odd choice.
Kylotan
Fixed the right way now.
bstpierre
A: 

The garbage collector can be flagged to clean up by setting the relevant object to None.

whatnick
+1  A: 

Can you really handle the Exception in connect()?

I think you should provide a MyClient.close() method, and write connect() like this:

def connect():
    try:
        c = MyClient(9989)
        print c.do_stuff()
    finally:
        c.close()

This is in complete analogy with file-like objects (and the with statement)

kaizer.se