From: Roger Pack <rogerpack2005@...>
Date: 2007-08-09T10:49:50+09:00
Subject: Re: deadlock in ThreadPool using backtick

Hmm.  I'm not a thread pro, but I'll give it a shot.
I don't think it's necessarily a problem with backtick, but with the 
code itself :)

first off

Justin Johnson wrote:
> Hello,
> 
> I'm using the ThreadPool class as defined in the Ruby Cookbook, recipe
> 20.7 and running on Windows XP with the One Click installer of Ruby
> 1.8.6 patch level 0.  Normally it works fine.  However, whenever I call
> a command using backticks I get a deadlock.  The script I'm running is
> below, followed by the output.  To see it succeed, comment out the
> backtick line and uncomment out the system line.
> 
> Does anyone know what the problem is?
> 
> Thanks,
> Justin
> 
> ##### BEGIN FILE ######
> require 'thread'
> 
> class ThreadPool
>   def initialize(max_size)
>     @pool = []
>     @max_size = max_size
>     @pool_mutex = Mutex.new
>     @pool_cv = ConditionVariable.new
>   end
> #---
>   def dispatch(*args)
>     Thread.new do
>       # Wait for space in the pool.
>       @pool_mutex.synchronize do
>         while @pool.size >= @max_size
>           print "Pool is full; waiting to run #{args.join(',')}...\n" if
> $DEBUG
>           # Sleep until some other thread calls @pool_cv.signal.
>           @pool_cv.wait(@pool_mutex)
>         end
>       end
> #---


>       @pool << Thread.current

The above line should be within the synchronize (above), or it might 
spawn more than @max_size threads (I think).
Replace with
      @pool_mutex.synchronize do
        while @pool.size >= @max_size
          print "Pool is full; waiting to run #{args.join(',')}...\n" if 
$DEBUG
          # Sleep until some other thread calls @pool_cv.signal.
          @pool_cv.wait(@pool_mutex)
        end
        @pool << Thread.current
      end

>       begin
>         yield(*args)
>       rescue => e
>         exception(self, e, *args)
>       ensure
>         @pool_mutex.synchronize do

In reality you don't need to synchronize here, as the @pool size/add to 
is synchronized elsewhere.  Taking this synch out may introduce new 
problems, but...I'm not sure what they are :)
My question is if the wait within the shutdown function will 'suck' 
signals away from processes, decreasing the number that can go at a 
time, if all are at the 'wait' phase.  Perhaps rethinking this code 
would be nice :)

-Roger


>           # Remove the thread from the pool.
>           @pool.delete(Thread.current)
>           # Signal the next waiting thread that there's a space in the
> pool.
>           @pool_cv.signal
>         end

>       end
>     end
>   end
> 
>   def shutdown
>     @pool_mutex.synchronize { @pool_cv.wait(@pool_mutex) until
> @pool.empty? }
>   end
> 
>   def exception(thread, exception, *original_args)
>     # Subclass this method to handle an exception within a thread.
>     puts "Exception in thread #{thread}: #{exception}"
>   end
> end
> #---
> 
> 
> 
> pool = ThreadPool.new 2
> begin
>   1.upto(200) do |i|
>     pool.dispatch do
>       puts "Job #{i} started."
>       `dir c:\\winnt > NUL`
>       #system("dir c:\\winnt > NUL")
>       puts "Job #{i} stopped."
>     end
>   end
> ensure
>   pool.shutdown
> end
> ##### END FILE ######
> 
> 
> ##### BEGIN OUTPUT ######
> C:\temp>ruby thread_test.rb
> Job 1 started.
> Job 2 started.
> Job 1 stopped.
> Job 2 stopped.
> Job 3 started.
> Job 6 started.
> Job 5 started.
> Job 4 started.
> Job 3 stopped.
> Job 6 stopped.
> Job 5 stopped.
> Job 4 stopped.
> deadlock 0x2b45008: sleep:-  - thread_test.rb:18
> deadlock 0x2b450d0: sleep:-  - thread_test.rb:18
> deadlock 0x2b45198: sleep:-  - thread_test.rb:18
> deadlock 0x2b45260: sleep:-  - thread_test.rb:18
> ... Skip many similar lines ...
> deadlock 0x2b5627c: sleep:-  - thread_test.rb:28
> deadlock 0x2b5636c: sleep:-  - thread_test.rb:28
> deadlock 0x27ac748: sleep:- (main) - thread_test.rb:39
> thread_test.rb:39:in `wait': Thread(0x27ac748): deadlock (fatal)
>         from thread_test.rb:39:in `shutdown'
>         from thread_test.rb:39:in `synchronize'
>         from thread_test.rb:39:in `shutdown'
>         from thread_test.rb:62
> 
> C:\temp>
> ##### END OUTPUT ######

There may also (I'm not positive about this), be a way that multiple 
signals may then occur very quickly, and only one processed.  Another 
possible problem, too.

GL
-Roger
-- 
Posted via http://www.ruby-forum.com/.