merge revision(s) 22011:

	* ext/thread/thread.c (rb_queue_pop, rb_queue_push): should not lock
	  mutex if got an exception while waiting, and should ensure unlocked
	  after signaled.  [ruby-dev:37545]


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

4 files changed, 67 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index 348a8fd6dc..fef9774ec8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+Mon Mar 23 19:00:59 2009  Nobuyoshi Nakada  <nobu@ruby-lang.org>
+
+	* ext/thread/thread.c (rb_queue_pop, rb_queue_push): should not lock
+	  mutex if got an exception while waiting, and should ensure unlocked
+	  after signaled.  [ruby-dev:37545]
+
 Mon Mar 23 18:12:47 2009  Nobuyoshi Nakada  <nobu@ruby-lang.org>
 
 	* eval.c (rb_thread_value): missed to change at r17874.  [ruby-core:17595]
diff --git a/ext/thread/thread.c b/ext/thread/thread.c
index ae08a61770..f59f704707 100644
--- a/ext/thread/thread.c
+++ b/ext/thread/thread.c
@@ -445,6 +445,12 @@ lock_mutex(Mutex *mutex)
 }
 
 static VALUE
+lock_mutex_call(VALUE mutex)
+{
+    return lock_mutex((Mutex *)mutex);
+}
+
+static VALUE
 rb_mutex_lock(VALUE self)
 {
     Mutex *mutex;
@@ -513,6 +519,12 @@ unlock_mutex(Mutex *mutex)
 }
 
 static VALUE
+unlock_mutex_call(VALUE mutex)
+{
+    return unlock_mutex((Mutex *)mutex);
+}
+
+static VALUE
 rb_mutex_unlock(VALUE self)
 {
     Mutex *mutex;
@@ -665,9 +677,18 @@ rb_condvar_alloc(VALUE klass)
  *
  */
 
+static void condvar_wakeup(Mutex *mutex);
+
 static void
 wait_condvar(ConditionVariable *condvar, Mutex *mutex)
 {
+    condvar_wakeup(mutex);
+    rb_ensure(wait_list, (VALUE)&condvar->waiting, lock_mutex_call, (VALUE)mutex);
+}
+
+static void
+condvar_wakeup(Mutex *mutex)
+{
     VALUE waking;
 
     rb_thread_critical = 1;
@@ -679,7 +700,6 @@ wait_condvar(ConditionVariable *condvar, Mutex *mutex)
     if (RTEST(waking)) {
 	wake_thread(waking);
     }
-    rb_ensure(wait_list, (VALUE)&condvar->waiting, relock_mutex, (VALUE)mutex);
 }
 
 static VALUE
@@ -762,6 +782,13 @@ signal_condvar(ConditionVariable *condvar)
 }
 
 static VALUE
+signal_condvar_call(VALUE condvar)
+{
+    signal_condvar((ConditionVariable *)condvar);
+    return Qundef;
+}
+
+static VALUE
 rb_condvar_signal(VALUE self)
 {
     ConditionVariable *condvar;
@@ -985,6 +1012,16 @@ rb_queue_num_waiting(VALUE self)
     return result;
 }
 
+static void
+wait_queue(ConditionVariable *condvar, Mutex *mutex)
+{
+    condvar_wakeup(mutex);
+    wait_list(&condvar->waiting);
+    lock_mutex(mutex);
+}
+
+static VALUE queue_pop_inner(VALUE arg);
+
 /*
  * Document-method: pop
  * call_seq: pop(non_block=false)
@@ -1000,7 +1037,6 @@ rb_queue_pop(int argc, VALUE *argv, VALUE self)
 {
     Queue *queue;
     int should_block;
-    VALUE result;
     Data_Get_Struct(self, Queue, queue);
 
     if (argc == 0) {
@@ -1018,15 +1054,21 @@ rb_queue_pop(int argc, VALUE *argv, VALUE self)
     }
 
     while (!queue->values.entries) {
-        wait_condvar(&queue->value_available, &queue->mutex);
+        wait_queue(&queue->value_available, &queue->mutex);
     }
 
-    result = shift_list(&queue->values);
+    return rb_ensure(queue_pop_inner, (VALUE)queue,
+		     unlock_mutex_call, (VALUE)&queue->mutex);
+}
+
+static VALUE
+queue_pop_inner(VALUE arg)
+{
+    Queue *queue = (Queue *)arg;
+    VALUE result = shift_list(&queue->values);
     if (queue->capacity && queue->values.size < queue->capacity) {
         signal_condvar(&queue->space_available);
     }
-    unlock_mutex(&queue->mutex);
-
     return result;
 }
 
@@ -1046,11 +1088,11 @@ rb_queue_push(VALUE self, VALUE value)
 
     lock_mutex(&queue->mutex);
     while (queue->capacity && queue->values.size >= queue->capacity) {
-        wait_condvar(&queue->space_available, &queue->mutex);
+        wait_queue(&queue->space_available, &queue->mutex);
     }
     push_list(&queue->values, value);
-    signal_condvar(&queue->value_available);
-    unlock_mutex(&queue->mutex);
+    rb_ensure(signal_condvar_call, (VALUE)&queue->value_available,
+	      unlock_mutex_call, (VALUE)&queue->mutex);
 
     return self;
 }
diff --git a/test/thread/test_thread.rb b/test/thread/test_thread.rb
index fe5fdeffda..7a4d110fea 100644
--- a/test/thread/test_thread.rb
+++ b/test/thread/test_thread.rb
@@ -115,5 +115,14 @@ class TC_Thread < Test::Unit::TestCase
 	# Now unlock. The mutex should be free, so Mutex#unlock should return nil
 	assert(! m.unlock)
     end
+
+    def test_queue_rescue
+        require "timeout"
+        queue = Queue.new
+        assert_raises(Timeout::Error) {Timeout.timeout(0.001) {queue.pop}}
+        queue.push(1)
+        assert_nothing_raised("[ruby-dev:37545]") {assert_equal(1, queue.pop)}
+        assert(queue.empty?)
+    end
 end
 
diff --git a/version.h b/version.h
index a249f8e2a2..a6b02f085a 100644
--- a/version.h
+++ b/version.h
@@ -2,7 +2,7 @@
 #define RUBY_RELEASE_DATE "2009-03-23"
 #define RUBY_VERSION_CODE 186
 #define RUBY_RELEASE_CODE 20090323
-#define RUBY_PATCHLEVEL 364
+#define RUBY_PATCHLEVEL 365
 
 #define RUBY_VERSION_MAJOR 1
 #define RUBY_VERSION_MINOR 8