summaryrefslogtreecommitdiff
path: root/thread_sync.c
diff options
context:
space:
mode:
authornormal <normal@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2018-08-18 06:33:49 +0000
committernormal <normal@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2018-08-18 06:33:49 +0000
commit647fc1227a4146ecbfeb0d59358abc8d99cd8ae6 (patch)
treea5c9bd35aaf8ca67b4ff3f27b56726f8a63cca05 /thread_sync.c
parent6e0d69e4a72c7a9d9cf5172bd9b74633d7259f9d (diff)
downloadruby-647fc1227a4146ecbfeb0d59358abc8d99cd8ae6.tar.gz
thread_sync.c (rb_mutex_synchronize): only unlock if we own the mutex
If an exception is raised inside Mutex#sleep (via ConditionVariable#wait), we cannot guarantee we can own the mutex in the ensure callback. However, who owns the mutex at that point does not matter. What matters is the Mutex is usable after an exception occurs. * thread_sync.c (rb_mutex_synchronize): only unlock if we own the mutex * spec/ruby/library/conditionvariable/wait_spec.rb: only test lock usability after thread kill. Who owns the lock at any particular moment is an implementation detail which we cannot easily guarantee. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64441 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
Diffstat (limited to 'thread_sync.c')
-rw-r--r--thread_sync.c74
1 files changed, 48 insertions, 26 deletions
diff --git a/thread_sync.c b/thread_sync.c
index 5e511af0db..f3d1ccb120 100644
--- a/thread_sync.c
+++ b/thread_sync.c
@@ -321,6 +321,38 @@ rb_mutex_owned_p(VALUE self)
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 *
rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th)
{
@@ -333,31 +365,7 @@ rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th)
err = "Attempt to unlock a mutex which is locked by another thread";
}
else {
- 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;
+ mutex_do_unlock(mutex, th);
}
return err;
@@ -497,6 +505,20 @@ mutex_sleep(int argc, VALUE *argv, VALUE self)
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:
* mutex.synchronize { ... } -> result of the block
@@ -509,7 +531,7 @@ VALUE
rb_mutex_synchronize(VALUE mutex, VALUE (*func)(VALUE arg), VALUE arg)
{
rb_mutex_lock(mutex);
- return rb_ensure(func, arg, rb_mutex_unlock, mutex);
+ return rb_ensure(func, arg, mutex_unlock_if_owned, mutex);
}
/*