# Flawed use of ConditionVariable **URL:** \ **Category:** ruby-talk\ **Created:** [10 January 2006 16:06 UTC](https://rubytalk.org/t/flawed-use-of-conditionvariable/24004 "2006-01-10T16:06:26Z")\ **Posts on this page:** 2\ **Page:** 1
**Author:** ![Mark\_Volkmann1](https://avatars.discourse-cdn.com/v4/letter/m/71e660/32.png) [@Mark\_Volkmann1](https://rubytalk.org/u/Mark_Volkmann1)\ **Post date:** [10 January 2006 16:06 UTC](https://rubytalk.org/t/flawed-use-of-conditionvariable/24004/1 "2006-01-10T16:06:26Z")
Example code I've seen that uses a ConditionVariable seems to have a timing flaw. For example, in Pickaxe 2, page 147 in the Player/Customer example, the player thread does   plays\_pending.wait\_while { playlist.empty? } This assumes that ok\_to\_shutdown gets set and detected in the previous line before wait\_while is called. If the timing works out such that ok\_to\_shutdown doesn't get set to true until after wait\_while is called, I believe it will hang forever. Isn't it the case that the customer thread could finish and the player thread could reach the call to wait\_while before ok\_to\_shutdown gets set to true? I think what needs to happen is what I show in the following example. Note how the wait\_while checks whether the producer thread is alive and how I do a condition.signal at the bottom when I know the producer thread has terminated ... which causes wait\_while to evaluate the condition again. The call to sleep at the end of the producer thread is critical to illustrating the problem. If I take that out then I can get lucky with the timing and don't need to test whether the producer thread is alive in wait\_while. Am I off my rocker? 😉 require 'monitor' > **···** > > #---------------------------------------------------------------------- > > class Widget >   attr\_accessor :id > >   def initialize(id) >     [self.id](http://self.id) = id >   end > >   def to\_s >     id >   end > end > > #---------------------------------------------------------------------- > > bin = [] > bin.extend MonitorMixin > condition = bin.new\_cond # type is MonitorMixin::ConditionVariable > > #---------------------------------------------------------------------- > > producer = Thread.new do >   widget\_id = 0 > >   5.times do >     widget\_id += 1 >     widget = Widget.new(widget\_id) >     puts "produced #{widget.to\_s}" > >     bin.synchronize do >       bin \<\< widget >       condition.signal # there is a widget to consume >     end >   end > >   # Delay termination of producer thread >   # to demonstrate need for another condition.signal. >   sleep 1 > end > > #---------------------------------------------------------------------- > > consumer = Thread.new do >   finished = false >   until (finished) do >     bin.synchronize do >       finished = (not producer.alive?) and bin.empty? >       break if finished > >       # The condition will be tested once, >       # and then again every time condition.signal is called. >       condition.wait\_while { producer.alive? and bin.empty? } > >       widget = bin.shift # removes first Widget from bin >       puts "consumed #{widget.to\_s}" if widget != nil >     end >   end > end > > #---------------------------------------------------------------------- > > producer.join > bin.synchronize do >   # Make consumer retest condition after producer terminates. >   condition.signal > end > consumer.join > > -- > R. Mark Volkmann > Partner, Object Computing, Inc. ---
**Author:** ![Robert](https://avatars.discourse-cdn.com/v4/letter/r/e95f7d/32.png) [@Robert](https://rubytalk.org/u/Robert)\ **Post date:** [10 January 2006 17:38 UTC](https://rubytalk.org/t/flawed-use-of-conditionvariable/24004/2 "2006-01-10T17:38:02Z")
Mark Volkmann wrote: > Example code I've seen that uses a ConditionVariable seems to have a > timing flaw. For example, in Pickaxe 2, page 147 in the > Player/Customer example, the player thread does > >   plays\_pending.wait\_while { playlist.empty? } > > This assumes that ok\_to\_shutdown gets set and detected in the previous > line before wait\_while is called. If the timing works out such that > ok\_to\_shutdown doesn't get set to true until after wait\_while is > called, I believe it will hang forever. I think theoretically you are right but in practice this code will be ok. Reason: playing a song will take significantly longer than thread "customer" needs to exit, main thread return from join and ok\_to\_shutdown set to true. So while the player is still playing the flag will be set to true and once the player is finished playing the last song it will hit the line that checks the global shutdown flag. In a Java VM this code would have the additional problem that access to ok\_to\_shutdown is not thread safe. The usual solution I choose for this is that I put as many terminators (special objects) into the queue (playlist here) that signal consumers that processing is over. That way no additional synchronization is needed and it's ensure that all consumer threads terminate. > Isn't it the case that the customer thread could finish and the player > thread could reach the call to wait\_while before ok\_to\_shutdown gets > set to true? > > I think what needs to happen is what I show in the following example. > Note how the wait\_while checks whether the producer thread is alive > and how I do a condition.signal at the bottom when I know the producer > thread has terminated ... which causes wait\_while to evaluate the > condition again. I wouldn't check for threads being alive. I'd rather synchronize accesses to ok\_to\_shutdown on playlist and notify when the state changes. Then the check would have to be moved into the while loop in the player thread. If I was to do this then I'd use class Queue. That's thread safe and you easily get what you want: require 'thread' TERM = Object.new.freeze QUEUE = Queue.new # init stuff consumer = Thread.new(QUEUE) do |queue|   until TERM == (song = queue.deq)     play(song)   end end until (req = get\_customer\_request).nil?   QUEUE.enc req end QUEUE.enc TERM consumer.join > The call to sleep at the end of the producer thread is critical to > illustrating the problem. If I take that out then I can get lucky with > the timing and don't need to test whether the producer thread is alive > in wait\_while. > > Am I off my rocker? 😉 No. Good eyes! Kind regards     robert