On Wed, 26 Mar 2008 07:32:27 +0900, "Adam Bender" <abender / gmail.com> wrote:
>   def push(value)
>     @values.synchronize do
>       @values.push value
>       @cond.signal
>     end
>     self
>   end
> 
>   def pop(timeout=nil)
>     ret = nil
>     @values.synchronize do
>       t = Time.now
>       @cond.wait(timeout)
>       puts "waited #{Time.now - t}"
>       ret = @values.shift unless @values.empty?
>     end
>     return ret
>   end

> The problem is that the wait() waits for timeout seconds, even when
> something is in the queue.

In this case, the problem is that you are always putting the thread to
sleep on the "pops" queue, even when there is already a pushed value
available on the "pushes" queue.

You need to check whether the "pushes" queue is empty and only sleep
when it is:

 def pop
   @values.synchronize do
     while @values.empty?
       @cond.wait
     end
     @values.shift
   end
 end

Note that I've used a while loop rather than a simple if test; this is
for several reasons, but mainly because it is possible for another
thread to "steal" a value from the "pops" queue before we finish
waking up.  Because of that, we have to check the predicate again after
@cond.wait returns, and potentially go back to sleep.

Implementing this in conjunction with timeout is harder, but possible:

 def pop(timeout = nil)
   if timeout
     deadline = Time.now + timeout
   else
     deadline = nil
   end
   @values.synchronize do
     while @values.empty?
       if deadline
         timeout = deadline - Time.now
         return nil if timeout <= 0
       end
       @cond.wait(timeout)
     end
     @values.shift
   end
 end

Since this uses the system clock rather than a monotonic timer, it does
still have a bug inasmuch as changing the system time can cause #pop to
wait for too long or too little, but Ruby 1.8 and 1.9 have that same bug
in their thread schedulers anyway.

-mental