[Tutor] sockets, files, threads

Danny Yoo dyoo at hkn.eecs.berkeley.edu
Wed Jan 19 08:13:09 CET 2005



On Tue, 18 Jan 2005, Marilyn Davis wrote:

>         while 1:
>             if log.level & log.calls:
>                 log.it("fd%d:py_daemon.py: Waiting ...", self.descriptor)
>             try:
>                 client_socket, client_addr = self.server_socket.accept()
>             except (EOFError, KeyboardInterrupt):
>                 self.close_up()
>             Spawn(client_socket).start()
>
> > The problem is that, as part of program flow, it appears to run after
> > the try block.  But in one particular case of program flow,
> > 'client_socket' will not be set to a valid value.  It is better to put
> > that statement a
>
> I don't understand.  Which particular case of program flow will
> 'client_socket' be invalid and yet make it through the socket.accept
> call?


Hi Marilyn,

If there is an EOFError or an KeyboardInterrupt, client_socket will
maintain the same value as the previous iteration through the whole loop.
What this potentially means is that, under strange situations, Spawn()
could be called on the same client_socket twice.

The issue is that the whole thing's in a 'while' loop, so we have to be
careful that the state from the previous loop iteration doesn't leak into
the current iteration.


[About using the Standard Library]

> And since then, please don't shoot me, but I don't immediately trust the
> modules.  I read them and see how many times they loop through the data,
> and how many copies of the data they put into memory -- and usually
> decide to write the simple things I need myself, looping zero times and
> keeping only one block in memory.

Hmm.. . Do you remember which Standard Library modules you were looking at
earlier?  Perhaps there was some funky stuff happening, in which case we
should try to fix it, so that no one else runs into the same problems.



> > ###
> > class FileReader(TokenReader):
> >     def __init__(self, file_socket):
> >         self.local_name = file_socket.local_name
> >         self.fread~er_name = '/tmp/fsf%s' % self.local_name
> >         file_lock.acquire()
> >         self.freader = open(self.freader_name, "w+")
> >         file_lock.release()
> > ###
> >
> > The locks around the open() calls are unnecessary: you do not need to
> > synchronize file opening here, as there's no way for another thread to
> > get into the same initializer of the same instance.
>
> But I think that I'm only wrapping the calls that create file
> descriptors, because those get trampled on.

I'm almost positive that the builtin open() function is thread-safe. Let
me check that really fast...

/*** Within Objects/fileobject.c: open_the_file() ***/
                if (NULL == f->f_fp && NULL != name) {
                        Py_BEGIN_ALLOW_THREADS
                        f->f_fp = fopen(name, mode);
                        Py_END_ALLOW_THREADS
                }
/******/

Hmmm!  If really depends if the underlying C's fopen() Standard C library
is thread safe.  This is true in modern versions of LIBC, so I don't think
there's anything to worry about here, unless you're using a very ancient
version of Unix.


> But, I did take out threading and the big error went away.  I'm done
> with threading, unless I see a big need one day.  I don't know what I'll
> tell students from here on.

I'd point them to John Ousterhout's article on "Why Threads are a Bad Idea
(For Most Purposes)":

    http://home.pacbell.net/ouster/threads.pdf



> > I thought about this a little bit more: there is one place where you
> > do need locks.

[text cut]

> But, notice that the call to:
>
>         threading.Thread.__init__(self, name = self.local_name)
>
> came after, so the Spawn.no manipulation always happens in the main
> thread.

You're right! I must have been completely delirious at that point.
*grin*



> One problem remains after removing the threading stuff.  I still get
> those pesky:
>
> close failed: [Errno 9] Bad file descriptor
>
> even though my logged openings and closings match up one-to-one.

Ok, then that's a good thing to know: since threading is off, we now know
that that error message has nothing to do with threads.


> Now then, I haven't added that line of code to each except clause yet
> because 1) it doesn't seem to cause any problem 2) it's a bunch of
> busy-work and 3) I'm hoping that, when your illness clears, you'll think
> of something less brute-force for me to do.

I'd recommend using traceback.format_exc().  It's infuriating to get an
error message like that, and not to know where in the world it's coming
from.

Python's default behavior for exceptions is to print out a good stack
trace: one of the best things about Python's default exception handler it
is that it gives us a local picture of the error.  For the most part, we
know around what line number we should be looking at.

When we write our own except handlers, the responsibility falls on us to
record good error messages.  If anything, our own debugging systems should
be even better than Python's.

So make the debugging easier on yourself: bite down and add the traceback
printouts.  At least we should try to isolate and localize where that
'close failed:  [Errno 9] Bad file descriptor' error message is coming
from.  I don't want to make any more flailing bug-hunting attempts until I
know where that thing is coming from.  *grin*.



> What about when I do an explicit call to a close inside a __del__.  Is
> that a bad idea?

I usually prefer to add a close() method to my objects that releases
resources, rather than __del__(), because it's more visible.

In a similar vein, in Spawn.run(), it might be a good idea to explicitely
call write_and_close() on our FileSocket instance.  For example, we can
add a try/finally in the body of the revised start()  method:

###
    def start(self):
        '''Given the command, provides the function to call.'''
        global TESTING
        function =  { 'acl_rcpt.py' : calls.acl_rcpt,
                      'route_mail.py' : calls.route_mail,
                      'route_errors.py' : calls.route_errors,
                      'doorman.py' : calls.doorman,
                      'doorman_errors.py' : calls.doorman_errors,
                      'is_known_to.py' : is_known_to
                      }
        if log.level & log.calls:
            log.it('fd%d: %s: %s', self.descriptor, self.exim_io.call,
                   self.exim_io.collector_name)
        try:
            function[self.exim_io.call].main(self.exim_io)
        finally:
            self.exim_io.write_and_close()
###

By using try/finally, we can guarantee that finalizers like
'write_and_close()' can be called at the end.

I couldn't tell when write_and_close() was getting called in the original
code; I suspected that each of the 'calls' dispatch functions was
individually responsible for calling write_and_close(), but I wasn't sure.


Best of wishes to you!



More information about the Tutor mailing list