1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00

Revert r64441

* This reverts commit 647fc1227a:
  "thread_sync.c (rb_mutex_synchronize): only unlock if we own the mutex"
* Let's try to preserve the semantics of always being locked inside
  Mutex#synchronize, even if an exception interrupts ConditionVariable#wait.
* As discussed on [Bug #14999].

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64448 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
This commit is contained in:
eregon 2018-08-18 13:52:53 +00:00
parent b5b5b28c65
commit c742050ea5
2 changed files with 44 additions and 56 deletions

View file

@ -23,15 +23,21 @@ describe "ConditionVariable#wait" do
th.join th.join
end end
it "the lock remains usable even if the thread is killed" do it "reacquires the lock even if the thread is killed" do
m = Mutex.new m = Mutex.new
cv = ConditionVariable.new cv = ConditionVariable.new
in_synchronize = false in_synchronize = false
owned = nil
th = Thread.new do th = Thread.new do
m.synchronize do m.synchronize do
in_synchronize = true in_synchronize = true
cv.wait(m) begin
cv.wait(m)
ensure
owned = m.owned?
$stderr.puts "\nThe Thread doesn't own the Mutex!" unless owned
end
end end
end end
@ -43,19 +49,24 @@ describe "ConditionVariable#wait" do
th.kill th.kill
th.join th.join
m.try_lock.should == true owned.should == true
m.unlock
end end
it "lock remains usable even if the thread is killed after being signaled" do it "reacquires the lock even if the thread is killed after being signaled" do
m = Mutex.new m = Mutex.new
cv = ConditionVariable.new cv = ConditionVariable.new
in_synchronize = false in_synchronize = false
owned = nil
th = Thread.new do th = Thread.new do
m.synchronize do m.synchronize do
in_synchronize = true in_synchronize = true
cv.wait(m) begin
cv.wait(m)
ensure
owned = m.owned?
$stderr.puts "\nThe Thread doesn't own the Mutex!" unless owned
end
end end
end end
@ -73,8 +84,7 @@ describe "ConditionVariable#wait" do
} }
th.join th.join
m.try_lock.should == true owned.should == true
m.unlock
end end
it "supports multiple Threads waiting on the same ConditionVariable and Mutex" do it "supports multiple Threads waiting on the same ConditionVariable and Mutex" do

View file

@ -321,38 +321,6 @@ rb_mutex_owned_p(VALUE self)
return owned; return owned;
} }
static void
mutex_do_unlock(rb_mutex_t *mutex, rb_thread_t *th)
{
struct sync_waiter *cur = 0, *next = 0;
rb_mutex_t **th_mutex = &th->keeping_mutexes;
VM_ASSERT(mutex->th == th);
mutex->th = 0;
list_for_each_safe(&mutex->waitq, cur, next, node) {
list_del_init(&cur->node);
switch (cur->th->status) {
case THREAD_RUNNABLE: /* from someone else calling Thread#run */
case THREAD_STOPPED_FOREVER: /* likely (rb_mutex_lock) */
rb_threadptr_interrupt(cur->th);
goto found;
case THREAD_STOPPED: /* probably impossible */
rb_bug("unexpected THREAD_STOPPED");
case THREAD_KILLED:
/* not sure about this, possible in exit GC? */
rb_bug("unexpected THREAD_KILLED");
continue;
}
}
found:
while (*th_mutex != mutex) {
th_mutex = &(*th_mutex)->next_mutex;
}
*th_mutex = mutex->next_mutex;
mutex->next_mutex = NULL;
}
static const char * static const char *
rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th) rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th)
{ {
@ -365,7 +333,31 @@ rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th)
err = "Attempt to unlock a mutex which is locked by another thread"; err = "Attempt to unlock a mutex which is locked by another thread";
} }
else { else {
mutex_do_unlock(mutex, th); struct sync_waiter *cur = 0, *next = 0;
rb_mutex_t **th_mutex = &th->keeping_mutexes;
mutex->th = 0;
list_for_each_safe(&mutex->waitq, cur, next, node) {
list_del_init(&cur->node);
switch (cur->th->status) {
case THREAD_RUNNABLE: /* from someone else calling Thread#run */
case THREAD_STOPPED_FOREVER: /* likely (rb_mutex_lock) */
rb_threadptr_interrupt(cur->th);
goto found;
case THREAD_STOPPED: /* probably impossible */
rb_bug("unexpected THREAD_STOPPED");
case THREAD_KILLED:
/* not sure about this, possible in exit GC? */
rb_bug("unexpected THREAD_KILLED");
continue;
}
}
found:
while (*th_mutex != mutex) {
th_mutex = &(*th_mutex)->next_mutex;
}
*th_mutex = mutex->next_mutex;
mutex->next_mutex = NULL;
} }
return err; return err;
@ -505,20 +497,6 @@ mutex_sleep(int argc, VALUE *argv, VALUE self)
return rb_mutex_sleep(self, timeout); return rb_mutex_sleep(self, timeout);
} }
static VALUE
mutex_unlock_if_owned(VALUE self)
{
rb_thread_t *th = GET_THREAD();
rb_mutex_t *mutex;
GetMutexPtr(self, mutex);
/* we may not own the mutex if an exception occured */
if (mutex->th == th) {
mutex_do_unlock(mutex, th);
}
return Qfalse;
}
/* /*
* call-seq: * call-seq:
* mutex.synchronize { ... } -> result of the block * mutex.synchronize { ... } -> result of the block
@ -531,7 +509,7 @@ VALUE
rb_mutex_synchronize(VALUE mutex, VALUE (*func)(VALUE arg), VALUE arg) rb_mutex_synchronize(VALUE mutex, VALUE (*func)(VALUE arg), VALUE arg)
{ {
rb_mutex_lock(mutex); rb_mutex_lock(mutex);
return rb_ensure(func, arg, mutex_unlock_if_owned, mutex); return rb_ensure(func, arg, rb_mutex_unlock, mutex);
} }
/* /*