From: Joel VanderWerf <vjoel@...>
Date: 2004-02-08T19:07:10+09:00
Subject: Re: Multi-threading lesson wanted

Tim Bates wrote:
> Hi all,
> I've not done a lot of multi-threaded programming before, and I now find 
> myself in the middle of writing a multi-threaded application. I'd 
...

You have some excitement ahead of you!

Here's one potential problem:

>       def allocate_handle
>         if @free_handles.empty?
...
>         else

At this point we've just decided that @free_handles is not empty.

>           return @free_handles.shift

But before the next line happens, another thread executes and steals the 
last handle. The return value is nil.

In the empty case, there is a less serious problem:

>       def allocate_handle
>         if @free_handles.empty?
>           if @handles.length < MAX_HANDLES

Two threads can get to this point at the same time. Then they both 
create a new handle, even though that might result in MAX_HANDLES+1 
handles. So, _very_ gradually, the size of the pool grows.

>             return new_handle

By the same token, this code:

>         while @free_handles.length > MAX_FREE_HANDLES
>           destroy_handle

could cause too many handles to be deleted: it's possible for N threads 
to be scheduled to check the condition, and then after all that checking 
is done, each calls destroy_handle. So you end up with too few free 
handles left.

There is also a performance problem with code like:

>             while @free_handles.empty?
>               sleep(1)
>             end

Rather than wake up each second and check for an available handle, the 
thread would be better off going to sleep indefinitely, and being 
wakened when a handle is available. This would save context-switches at 
a rate of twice per waiting thread per second. And it would reduce delay 
from 0.5sec average to (probably) milliseconds.

On the positive side, this code won't ever give the same handle to two 
threads, because #shift is atomic, as far as ruby threads are concerned.

The construct that would probably apply best here is the Queue in 
thread.rb. It would replace @free_handles (just create 20 handles and 
put them on the queue, use #pop and #push to wait for and to release 
handles). It would solve the performance problem as described (a thread 
goes to sleep waiting for a handle, and is woken by another thread when 
there is a handle available in the queue). It would also avoid the race 
condition inherent in checking empty? and then shifting.

However, it won't help with the MAX_FREE_HANDLES logic that you've 
designed. You would probably have to use Thread.critical for that, so 
that you can check the size of the queue and be certain that it isn't 
changing while you decide to destroy a handle or not.