On Tue, 18 Jan 2005, Danny Yoo wrote:

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!