From: Yusuke Endoh <redmine@...>
Date: 2010-05-03T22:15:11+09:00
Subject: [ruby-core:29971] [Bug #3212](Open) ConditionVariable may become inconsistent for interrupted threads

Issue #3212 has been updated by Yusuke Endoh.

Status changed from Rejected to Open

Hi,

> Scenario 1
> ----------
> 1) Thread 'a' gets a spurious wakeup
> 2) Thread 'a' gets back to wait() because it is in an until loop. @waiters contains 'a' two times.
> 3) Thread 'b' wakes 'a' up
> 4) Thread 'a' leaves the synchronization section. 'a' is still listed in @waiters (the bug I am reporting)
> 5) Later on, 'a' goes back to sleep (for whatever reason)
> 6) Thread 'b' calls CV#signal and wakes up 'a', since 'a' is still in @waiters

You are right!  I never thought of this scenario.
I think CV#wait should add the thread to @waiter unless it is not
contained yet.


> Scenario 2
> ----------
> 1) Thread 'a' gets a spurious wakeup
> 2) Thread 'a' gets back to sleep *not using CV#wait" (using mutex#sleep or Thread.sleep, for whatever reason)
> 3) Thread 'b' calls CV#signal, which wakes 'a' up since 'a' is still in the @waiters list.

#2 of this scenario is impossible, because the corresponding
predicate is still false.  By the while loop, Thread 'a' must get
back to CV#wait.  But I doubt if I'm right.


reproducing script:
  require "thread"

  m = Mutex.new
  cv = ConditionVariable.new
  flag = false

  # three waiting threads
  a, b, c = (0..2).map do |i|
    Thread.new do
      m.synchronize do
        2.times do
          cv.wait(m) until flag
          p "Thread #{i} wakeup"
          flag = false
        end
      end
      n = sleep 3
      p "Thread #{i} wrong wakeup" if n < 3
    end
  end

  # spurious wakeup
  sleep 0.5
  c.run
  sleep 0.5
  c.run

  # wake the threads up
  6.times do
    sleep 0.5
    m.synchronize { flag = true; cv.signal }
  end

  a.join
  b.join
  c.join


expected transcript:
  "Thread 0 wakeup"
  "Thread 1 wakeup"
  "Thread 2 wakeup"
  "Thread 0 wakeup"
  "Thread 1 wakeup"
  "Thread 2 wakeup"

actual transcript:
  "Thread 0 wakeup"
  "Thread 1 wakeup"
  "Thread 2 wakeup"
  "Thread 2 wakeup"
  "Thread 2 wrong wakeup"
  "Thread 0 wakeup"
  t.rb:35:in `join': deadlock detected (fatal)
          from t.rb:35:in `<main>'


diff --git a/lib/thread.rb b/lib/thread.rb
index f3831a7..d3b789b 100644
--- a/lib/thread.rb
+++ b/lib/thread.rb
@@ -66,7 +66,7 @@ class ConditionVariable
     begin
       # TODO: mutex should not be used
       @waiters_mutex.synchronize do
-        @waiters.push(Thread.current)
+        @waiters.push(Thread.current) unless @waiters.include?(Thread.current)
       end
       mutex.sleep timeout
     end

-- 
Yusuke Endoh <mame@tsg.ne.jp>
----------------------------------------
http://redmine.ruby-lang.org/issues/show/3212

----------------------------------------
http://redmine.ruby-lang.org