flawed use of ConditionVariable

[Mark Volkmann]

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 =3D id
end

def to_s
id
end
end

#----------------------------------------------------------------------

bin =3D []
bin.extend MonitorMixin
condition =3D bin.new_cond # type is MonitorMixin::ConditionVariable

#----------------------------------------------------------------------

producer =3D Thread.new do
widget_id =3D 0

5.times do
widget_id +=3D 1
widget =3D 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 =3D Thread.new do
finished =3D false
until (finished) do
bin.synchronize do
finished =3D (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 =3D bin.shift # removes first Widget from bin
puts "consumed #{widget.to_s}" if widget !=3D nil
end
end
end

#----------------------------------------------------------------------

producer.join
bin.synchronize do
# Make consumer retest condition after producer terminates.
condition.signal
end
consumer.join

 

[Robert Klemme]

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