From: Robert Klemme <bob.news@...>
Date: 2006-01-11T02:38:02+09:00
Subject: Re: flawed use of ConditionVariable

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