Skip to content

Commit

Permalink
Remove from waiter in Mutex#lock with ensure when calling rb_schedule…
Browse files Browse the repository at this point in the history
…r_block()

* Previously this could lead to an invalid waiter entry and then trying
  to wake up that waiter would result in various issues in rb_mutex_unlock_th().
  • Loading branch information
eregon committed Sep 20, 2020
1 parent 73a626c commit 6987c89
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 6 deletions.
32 changes: 32 additions & 0 deletions test/fiber/test_mutex.rb
Expand Up @@ -70,6 +70,38 @@ def test_mutex_thread
thread.join
end

def test_mutex_fiber_raise
mutex = Mutex.new
ran = false

main = Thread.new do
mutex.lock

thread = Thread.new do
scheduler = Scheduler.new
Thread.current.scheduler = scheduler

f = Fiber.schedule do
assert_raise_with_message(RuntimeError, "bye") do
assert_same scheduler, Thread.scheduler
mutex.lock
end
ran = true
end

Fiber.schedule do
f.raise "bye"
end
end

thread.join
end

main.join # causes mutex to be released
assert_equal false, mutex.locked?
assert_equal true, ran
end

def test_condition_variable
mutex = Mutex.new
condition = ConditionVariable.new
Expand Down
19 changes: 13 additions & 6 deletions thread_sync.c
Expand Up @@ -214,18 +214,17 @@ VALUE
rb_mutex_trylock(VALUE self)
{
rb_mutex_t *mutex = mutex_ptr(self);
VALUE locked = Qfalse;

if (mutex->fiber == 0) {
rb_fiber_t *fiber = GET_EC()->fiber_ptr;
rb_thread_t *th = GET_THREAD();
mutex->fiber = fiber;
locked = Qtrue;

mutex_locked(th, self);
return Qtrue;
}

return locked;
return Qfalse;
}

/*
Expand All @@ -246,6 +245,16 @@ mutex_owned_p(rb_fiber_t *fiber, rb_mutex_t *mutex)
}
}

static VALUE call_rb_scheduler_block(VALUE mutex) {
return rb_scheduler_block(rb_thread_current_scheduler(), mutex);
}

static VALUE remove_from_mutex_lock_waiters(VALUE arg) {
struct list_node *node = (struct list_node*)arg;
list_del(node);
return Qnil;
}

static VALUE
do_mutex_lock(VALUE self, int interruptible_p)
{
Expand Down Expand Up @@ -276,9 +285,7 @@ do_mutex_lock(VALUE self, int interruptible_p)
if (scheduler != Qnil) {
list_add_tail(&mutex->waitq, &w.node);

rb_scheduler_block(scheduler, self);

list_del(&w.node);
rb_ensure(call_rb_scheduler_block, self, remove_from_mutex_lock_waiters, (VALUE)&w.node);

if (!mutex->fiber) {
mutex->fiber = fiber;
Expand Down

0 comments on commit 6987c89

Please sign in to comment.