From 070557afc4ca83876b951fe090806b59e3867ae5 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Thu, 6 Feb 2020 09:14:40 +0900 Subject: Distinguish signal and timeout [Bug #16608] --- ext/monitor/monitor.c | 4 ++-- test/monitor/test_monitor.rb | 2 +- test/ruby/test_thread_cv.rb | 8 ++++++-- thread.c | 6 ++++-- thread_sync.c | 18 +++++++++++------- 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/ext/monitor/monitor.c b/ext/monitor/monitor.c index 00d663195f..10209cf2aa 100644 --- a/ext/monitor/monitor.c +++ b/ext/monitor/monitor.c @@ -149,8 +149,8 @@ monitor_wait_for_cond_body(VALUE v) struct wait_for_cond_data *data = (struct wait_for_cond_data *)v; struct rb_monitor *mc = monitor_ptr(data->monitor); // cond.wait(monitor.mutex, timeout) - rb_funcall(data->cond, rb_intern("wait"), 2, mc->mutex, data->timeout); - return Qtrue; + VALUE signaled = rb_funcall(data->cond, rb_intern("wait"), 2, mc->mutex, data->timeout); + return RTEST(signaled) ? Qtrue : Qfalse; } static VALUE diff --git a/test/monitor/test_monitor.rb b/test/monitor/test_monitor.rb index 3eceee7b2e..8ff6d006df 100644 --- a/test/monitor/test_monitor.rb +++ b/test/monitor/test_monitor.rb @@ -294,7 +294,7 @@ class TestMonitor < Test::Unit::TestCase @monitor.synchronize do assert_equal("foo", c) result3 = cond.wait(0.1) - assert_equal(true, result3) # wait always returns true in Ruby 1.9 + assert_equal(false, result3) assert_equal("foo", c) queue3.enq(nil) result4 = cond.wait diff --git a/test/ruby/test_thread_cv.rb b/test/ruby/test_thread_cv.rb index 5f19b85bc0..812c4221bc 100644 --- a/test/ruby/test_thread_cv.rb +++ b/test/ruby/test_thread_cv.rb @@ -16,6 +16,7 @@ class TestThreadConditionVariable < Test::Unit::TestCase mutex = Thread::Mutex.new condvar = Thread::ConditionVariable.new result = [] + woken = nil mutex.synchronize do t = Thread.new do mutex.synchronize do @@ -25,11 +26,12 @@ class TestThreadConditionVariable < Test::Unit::TestCase end result << 0 - condvar.wait(mutex) + woken = condvar.wait(mutex) result << 2 t.join end assert_equal([0, 1, 2], result) + assert(woken) end def test_condvar_wait_exception_handling @@ -140,11 +142,12 @@ INPUT condvar = Thread::ConditionVariable.new timeout = 0.3 locked = false + woken = true t0 = Time.now mutex.synchronize do begin - condvar.wait(mutex, timeout) + woken = condvar.wait(mutex, timeout) ensure locked = mutex.locked? end @@ -154,6 +157,7 @@ INPUT assert_operator(timeout*0.9, :<, t) assert(locked) + assert_nil(woken) end def test_condvar_nolock diff --git a/thread.c b/thread.c index 47bbb4257c..0f6838e141 100644 --- a/thread.c +++ b/thread.c @@ -132,7 +132,7 @@ rb_thread_local_storage(VALUE thread) return rb_ivar_get(thread, idLocals); } -static void sleep_hrtime(rb_thread_t *, rb_hrtime_t, unsigned int fl); +static int sleep_hrtime(rb_thread_t *, rb_hrtime_t, unsigned int fl); static void sleep_forever(rb_thread_t *th, unsigned int fl); static void rb_thread_sleep_deadly_allow_spurious_wakeup(VALUE blocker); static int rb_threadptr_dead(rb_thread_t *th); @@ -1479,7 +1479,7 @@ hrtime_update_expire(rb_hrtime_t *timeout, const rb_hrtime_t end) } COMPILER_WARNING_POP -static void +static int sleep_hrtime(rb_thread_t *th, rb_hrtime_t rel, unsigned int fl) { enum rb_thread_status prev_status = th->status; @@ -1495,8 +1495,10 @@ sleep_hrtime(rb_thread_t *th, rb_hrtime_t rel, unsigned int fl) break; if (hrtime_update_expire(&rel, end)) break; + woke = 1; } th->status = prev_status; + return woke; } void diff --git a/thread_sync.c b/thread_sync.c index 894235e9e1..44290136b3 100644 --- a/thread_sync.c +++ b/thread_sync.c @@ -533,14 +533,15 @@ rb_mutex_wait_for(VALUE time) { rb_hrtime_t *rel = (rb_hrtime_t *)time; /* permit spurious check */ - sleep_hrtime(GET_THREAD(), *rel, 0); - return Qnil; + if (sleep_hrtime(GET_THREAD(), *rel, 0)) return Qtrue; + return Qfalse; } VALUE rb_mutex_sleep(VALUE self, VALUE timeout) { struct timeval t; + VALUE woken = Qtrue; if (!NIL_P(timeout)) { t = rb_time_interval(timeout); @@ -560,18 +561,19 @@ rb_mutex_sleep(VALUE self, VALUE timeout) } else { rb_hrtime_t rel = rb_timeval2hrtime(&t); - rb_ensure(rb_mutex_wait_for, (VALUE)&rel, mutex_lock_uninterruptible, self); + woken = rb_ensure(rb_mutex_wait_for, (VALUE)&rel, mutex_lock_uninterruptible, self); } } RUBY_VM_CHECK_INTS_BLOCKING(GET_EC()); + if (!woken) return Qnil; time_t end = time(0) - beg; return TIMET2NUM(end); } /* * call-seq: - * mutex.sleep(timeout = nil) -> number + * mutex.sleep(timeout = nil) -> number or nil * * Releases the lock and sleeps +timeout+ seconds if it is given and * non-nil or forever. Raises +ThreadError+ if +mutex+ wasn't locked by @@ -582,6 +584,8 @@ rb_mutex_sleep(VALUE self, VALUE timeout) * * Note that this method can wakeup without explicit Thread#wakeup call. * For example, receiving signal and so on. + * + * Returns the slept time in seconds if woken up, or +nil+ if timed out. */ static VALUE mutex_sleep(int argc, VALUE *argv, VALUE self) @@ -1483,6 +1487,8 @@ do_sleep(VALUE args) * * If +timeout+ is given, this method returns after +timeout+ seconds passed, * even if no other thread doesn't signal. + * + * Returns the slept result on +mutex+. */ static VALUE @@ -1502,9 +1508,7 @@ rb_condvar_wait(int argc, VALUE *argv, VALUE self) }; list_add_tail(&cv->waitq, &sync_waiter.node); - rb_ensure(do_sleep, (VALUE)&args, delete_from_waitq, (VALUE)&sync_waiter); - - return self; + return rb_ensure(do_sleep, (VALUE)&args, delete_from_waitq, (VALUE)&sync_waiter); } /* -- cgit v1.2.3