Project

General

Profile

Backport #1471

"Mutual join" deadlock detection faulty in 1.8.6 and 1.8.7

Added by JohnCarter (John Carter) about 11 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
[ruby-core:23457]

Description

=begin
If a third thread is involved, the code in eval.c in function
rb_thread_join0 to detect a mutual join dead lock is faulty.

This effects ruby 1.8.6 p368 and ruby 1.8.7 p 160 but not ruby 1.9.1 p
129

What seems to be happening is a sequence like this....

main thread grabs mutex

thread two blocks waiting for mutex

thread one blocks waiting for mutex and th->join is set to the main thread.

the main thread unlocks, which wakes thread two. Thread two now has mutex, thread one is still blocked.

the main thread attempts to join thread one, rb_thread_join0 notes
that thread one is in a WAIT_JOIN waitfor state AND th->join still
points at the main thread and (erroneously) reports deadlock.

ruby-1.8.7-p160/eval.c================================================
rb_thread_join0(th, limit)
rb_thread_t th;
double limit;
{
enum rb_thread_status last_status = THREAD_RUNNABLE;

 if (rb_thread_critical) rb_thread_deadlock();
 if (!rb_thread_dead(th)) {
     if (th == curr_thread)
         rb_raise(rb_eThreadError, "thread 0x%lx tried to join itself",
                  th->thread);
     if ((th->wait_for & WAIT_JOIN) && th->join == curr_thread)
         rb_raise(rb_eThreadError, "Thread#join: deadlock 0x%lx - mutual join(0x%lx)",
                  curr_thread->thread, th->thread);

======================================================================
Affected versions.

/opt/ruby/ruby-1.8.6-p368/bin/ruby -v
ruby 1.8.6 (2009-03-31 patchlevel 368) [i686-linux]

/opt/ruby/ruby-1.8.7-p160/bin/ruby -v
ruby 1.8.7 (2009-04-08 patchlevel 160) [i686-linux]

Here is a test script to trigger it....
=mutual_join_bug.rb============================================================
require 'thread'

m = Mutex.new
m.lock
wt2 = Thread.new do
m.lock
sleep 2
m.unlock
end

# Ensure wt2 is waiting on m
sleep 0.1

wt1 = Thread.new do
m.lock
m.unlock
end
# Ensure wt1 is waiting on m
sleep 0.1

# Give it to wt2
m.unlock

wt1.join
======================================================================

Output...
/opt/ruby/ruby-1.8.7-p160/bin/ruby -w mutual_join_bug.rb mutual_join_bug.rb:24:in `join': Thread#join: deadlock 0xb7daa1bc - mutual join(0xb7d9bc48) (ThreadError)
from mutual_join_bug.rb:24

The following patch "fixes" it...
======================================================================
--- eval.c_orig 2009-03-23 22:28:31.000000000 +1300
+++ eval.c 2009-05-15 15:15:34.141270568 +1200
@@ -11426,9 +11426,13 @@
if (th == curr_thread)
rb_raise(rb_eThreadError, "thread 0x%lx tried to join itself",
th->thread);
+/* This test for mutual join deadlock is faulty in that

  • it doesn't allow for the possibility that a third thread
  • may now hold the mutex. if ((th->wait_for & WAIT_JOIN) && th->join == curr_thread) rb_raise(rb_eThreadError, "Thread#join: deadlock 0x%lx - mutual join(0x%lx)", curr_thread->thread, th->thread); +*/ if (curr_thread->status == THREAD_TO_KILL) last_status = THREAD_TO_KILL; if (limit == 0) return Qfalse; ======================================================================

But I'm not sure it is the correct fix. I'm still experience some
strangeness which I haven't pinned down yet.

I would seem to me that the correct fix is when a mutex is unlocked,
and then locked by some other thread, the th->join pointers of all
threads waiting on that mutex should be updated to point at the thread
now holding the mutex.
=end


Files

mutual_join_bug.rb (261 Bytes) mutual_join_bug.rb Test script. JohnCarter (John Carter), 05/15/2009 05:19 PM
ruby_1_8_7_reset_waitfor_on_dead_threads.patch (1.13 KB) ruby_1_8_7_reset_waitfor_on_dead_threads.patch Reset wait_for when joining to a dead thread & check consistency on rb_thread_set_join JohnCarter (John Carter), 05/19/2009 02:39 PM
ruby_1_8_7_thread_adjust_join.patch (737 Bytes) ruby_1_8_7_thread_adjust_join.patch Move adjust join out of wake_on into unlock_mutex_inner JohnCarter (John Carter), 05/19/2009 02:39 PM

Also available in: Atom PDF