[Tutor] sockets, files, threads

[Danny Yoo]

On Tue, 18 Jan 2005, Danny Yoo wrote:

> In fact, as far as I can tell, none of the Spawn() threads are
> communicating with each other.  As long as your threads are working
> independently of each other --- and as long as they are not writing to
> global variables --- you do not need locks.
>
> In summary, a lot of that locking code isn't doing anything, and may
> actually be a source of problems if we're not careful.


Hi Marilyn,

I thought about this a little bit more: there is one place where you do
need locks.  Locking needs to be done around Spawn's setting of its 'no'
class attribute:

###
class Spawn(threading.Thread):
    no = 1
    def __init__(self, client_socket):
        Spawn.no = Spawn.no + 2 % 30000
        ## some code later...
        self.local_name = str(Spawn.no)
####

The problem is that it's possible that two Spawn threads will be created
with the same local_name, because they are both using a shared resource:
the class attribute 'no'.  Two thread executions can interleave.  Let's
draw it out.



Imagine we bring two threads t1 and t2 up, and that Spawn.no is set to 1.
Now, as both are initializing, imagine that t1 runs it's initializer

### t1 ###
    def __init__(self, client_socket):
        Spawn.no = Spawn.no + 2 % 30000
##########

and then passes up control.

At this point, Spawn.no = 3.  Imagine that t2 now starts up:

### t2 ###
    def __init__(self, client_socket):
        Spawn.no = Spawn.no + 2 % 30000
        ### some code later
        self.local_name = str(Spawn.no)
##########

Now Spawn.no = 5, and that's what t2 uses to initialize itself.  When t1
starts off where it left off,

### t1 ###
        ### some code later
        self.local_name = str(Spawn.no)
##########

it too uses Spawn.no, which is still set to 5.

That's the scenario we need to prevent.  Spawn.no is a shared resource: it
can have only one value, and unfortunately, both threads can use the same
value for their own local_names.  This is a major bug, because much of
your code assumes that the local_name attribute is unique.



This is a situation where synchronization locks are appropriate and
necessary.  We'll want to use locks around the whole access to Spawn.no.
Something like:

###
class Spawn(threading.Thread):
    no = 1
    no_lock = threading.Lock()
    def __init__(self, client_socket):
        no_lock.acquire()
        try:
            Spawn.no = Spawn.no + 2 % 30000
             ## ... rest of initializer body is here
        finally:
            no_lock.release()
###

should do the trick.  I'm being a little paranoid by using the try/finally
block, but a little paranoia might be justified here.  *grin*


We need to be careful of how locks work.  The following code is broken:

###
class Spawn(threading.Thread):
    no = 1
    no_lock = threading.Lock()
    def __init__(self, client_socket):    ## buggy
        no_lock.acquire()
        Spawn.no = Spawn.no + 2 % 30000
        no_lock.release()

        self.descriptor = client_socket.fileno()
        self.client_socket = client_socket

        no_lock.acquire()
        self.local_name = str(Spawn.no)
        no_lock.release()
        ### ...
###

This code suffers from the same bug as the original code: two threads can
come in, interleave, and see the same Spawn.no.  It's not enough to put
lock acquires() and releases() everywhere: we do have to use them
carefully or else lose the benefit that they might provide.


Anyway, I hope this helps!