* merge -r 12066:12069

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_1_8_6@12077 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

4 files changed, 102 insertions, 13 deletions

diff --git a/ChangeLog b/ChangeLog
index 3c3ac3dfdf..4b11892bc0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,25 @@
+Fri Mar 16 16:33:58 2007  Akinori MUSHA  <knu@iDaemons.org>
+
+	* ext/thread/thread.c (unlock_mutex_inner): Make sure that the
+	  given mutex is actually owned by the caller; submitted by:
+	  Sylvain Joyeux <sylvain.joyeux AT m4x.org> in [ruby-core:10598].
+
+Fri Mar 16 16:21:35 2007  Akinori MUSHA  <knu@iDaemons.org>
+
+	* ext/thread/thread.c (wait_condvar, lock_mutex): Fix a problem in
+	  ConditionVariable#wait that occurs when two threads that are
+	  trying to access the condition variable are also in concurrence
+	  for the given mutex; submitted by: Sylvain Joyeux
+	  <sylvain.joyeux AT m4x.org> and MenTaLguY <mental AT rydia.net>
+	  in [ruby-core:10598].
+
+Fri Mar 16 16:17:27 2007  Akinori MUSHA  <knu@iDaemons.org>
+
+	* test/thread/test_thread.rb: Add a test script for the `thread'
+	  library.  This should result in failure as of now with
+	  ext/thread; submitted by: Sylvain Joyeux <sylvain.joyeux AT
+	  m4x.org> in [ruby-core:10598].
+
 Wed Mar 14 12:30:00 2007  Shigeo Kobayashi  <shigeo@tinyforest.jp>
 
 	* ext/bigdecimal/bigdecimal.c: BigDecimal("-.31") is now
diff --git a/ext/thread/thread.c b/ext/thread/thread.c
index e9dde37a18..d617185a26 100644
--- a/ext/thread/thread.c
+++ b/ext/thread/thread.c
@@ -390,7 +390,7 @@ rb_mutex_try_lock(VALUE self)
  *
  */
 
-static void
+static VALUE
 lock_mutex(Mutex *mutex)
 {
     VALUE current;
@@ -405,6 +405,7 @@ lock_mutex(Mutex *mutex)
     mutex->owner = current; 
 
     rb_thread_critical = 0;
+    return Qnil;
 }
 
 static VALUE
@@ -429,8 +430,13 @@ unlock_mutex_inner(Mutex *mutex)
     VALUE waking;
 
     if (!RTEST(mutex->owner)) {
-        return Qundef;
+	rb_raise(rb_eThreadError, "not owner");
     }
+
+    if (mutex->owner != rb_thread_current()) {
+	rb_raise(rb_eThreadError, "not owner");
+    }
+
     mutex->owner = Qnil;
     waking = wake_one(&mutex->waiting);
 
@@ -623,18 +629,12 @@ static void
 wait_condvar(ConditionVariable *condvar, Mutex *mutex)
 {
     rb_thread_critical = 1;
-    if (!RTEST(mutex->owner)) {
-        rb_thread_critical = 0;
-        return;
-    }
-    if (mutex->owner != rb_thread_current()) {
+    if (rb_thread_current() != mutex->owner) {
         rb_thread_critical = 0;
-        rb_raise(rb_eThreadError, "Not owner");
+        rb_raise(rb_eThreadError, "not owner of the synchronization mutex");
     }
-    mutex->owner = Qnil;
-    wait_list(&condvar->waiting);
-
-    lock_mutex(mutex);
+    unlock_mutex_inner(mutex);
+    rb_ensure(wait_list, (VALUE)&condvar->waiting, lock_mutex, (VALUE)mutex);
 }
 
 static VALUE
diff --git a/test/thread/test_thread.rb b/test/thread/test_thread.rb
new file mode 100644
index 0000000000..abdcebffcf
--- /dev/null
+++ b/test/thread/test_thread.rb
@@ -0,0 +1,67 @@
+require 'thread'
+require 'test/unit'
+
+class TC_Thread < Test::Unit::TestCase
+    def setup
+	Thread.abort_on_exception = true
+    end
+    def teardown
+	Thread.abort_on_exception = false
+    end
+    def test_condvar
+	mutex = Mutex.new
+	condvar = ConditionVariable.new
+	result = []
+	mutex.synchronize do
+	    t = Thread.new do
+		mutex.synchronize do
+		    result << 1
+		    condvar.signal
+		end
+	    end
+	
+	    result << 0
+	    condvar.wait(mutex)
+	    result << 2
+	    t.join
+	end
+	assert_equal([0, 1, 2], result)
+    end
+
+    def test_condvar_wait_not_owner
+	mutex = Mutex.new
+	condvar = ConditionVariable.new
+
+	assert_raises(ThreadError) { condvar.wait(mutex) }
+    end
+
+    def test_condvar_wait_exception_handling
+	# Calling wait in the only thread running should raise a ThreadError of
+	# 'stopping only thread'
+	mutex = Mutex.new
+	condvar = ConditionVariable.new
+
+	Thread.abort_on_exception = false
+
+	locked = false
+	thread = Thread.new do
+	    mutex.synchronize do
+		begin
+		    condvar.wait(mutex)
+		rescue Exception
+		    locked = mutex.locked?
+		    raise
+		end
+	    end
+	end
+
+	while !thread.stop?
+	    sleep(0.1)
+	end
+
+	thread.raise Interrupt, "interrupt a dead condition variable"
+	assert_raises(Interrupt) { thread.value }
+	assert(locked)
+    end
+end
+
diff --git a/version.h b/version.h
index 7c6e0e8004..7613e060f5 100644
--- a/version.h
+++ b/version.h
@@ -2,7 +2,7 @@
 #define RUBY_RELEASE_DATE "2007-03-16"
 #define RUBY_VERSION_CODE 186
 #define RUBY_RELEASE_CODE 20070316
-#define RUBY_PATCHLEVEL 1
+#define RUBY_PATCHLEVEL 2
 
 #define RUBY_VERSION_MAJOR 1
 #define RUBY_VERSION_MINOR 8