ruby-changes:66888
From: Nobuyoshi <ko1@a...>
Date: Mon, 26 Jul 2021 05:09:19 +0900 (JST)
Subject: [ruby-changes:66888] 070557afc4 (master): Distinguish signal and timeout [Bug #16608]
https://git.ruby-lang.org/ruby.git/commit/?id=070557afc4 From 070557afc4ca83876b951fe090806b59e3867ae5 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada <nobu@r...> 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 00d6631..10209cf 100644 --- a/ext/monitor/monitor.c +++ b/ext/monitor/monitor.c @@ -149,8 +149,8 @@ monitor_wait_for_cond_body(VALUE v) https://github.com/ruby/ruby/blob/trunk/ext/monitor/monitor.c#L149 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 3eceee7..8ff6d00 100644 --- a/test/monitor/test_monitor.rb +++ b/test/monitor/test_monitor.rb @@ -294,7 +294,7 @@ class TestMonitor < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/monitor/test_monitor.rb#L294 @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 5f19b85..812c422 100644 --- a/test/ruby/test_thread_cv.rb +++ b/test/ruby/test_thread_cv.rb @@ -16,6 +16,7 @@ class TestThreadConditionVariable < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_thread_cv.rb#L16 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_thread_cv.rb#L26 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_thread_cv.rb#L142 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_thread_cv.rb#L157 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 47bbb42..0f6838e 100644 --- a/thread.c +++ b/thread.c @@ -132,7 +132,7 @@ rb_thread_local_storage(VALUE thread) https://github.com/ruby/ruby/blob/trunk/thread.c#L132 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) https://github.com/ruby/ruby/blob/trunk/thread.c#L1479 } 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) https://github.com/ruby/ruby/blob/trunk/thread.c#L1495 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 894235e..4429013 100644 --- a/thread_sync.c +++ b/thread_sync.c @@ -533,14 +533,15 @@ rb_mutex_wait_for(VALUE time) https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L533 { 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) https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L561 } 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) https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L584 * * 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) https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L1487 * * 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) https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L1508 }; 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.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/