Skip to content

Commit a8c2d5e

Browse files
authored
Ensure fiber scheduler re-acquires mutex when interrupted from sleep. (ruby#12158)
[Bug #20907]
1 parent 3199766 commit a8c2d5e

File tree

2 files changed

+61
-27
lines changed

2 files changed

+61
-27
lines changed

test/fiber/test_scheduler.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,4 +182,32 @@ def test_deadlock
182182
thread.join
183183
signaller.join
184184
end
185+
186+
def test_condition_variable
187+
condition_variable = ::Thread::ConditionVariable.new
188+
mutex = ::Thread::Mutex.new
189+
190+
error = nil
191+
192+
thread = Thread.new do
193+
Thread.current.report_on_exception = false
194+
195+
scheduler = Scheduler.new
196+
Fiber.set_scheduler scheduler
197+
198+
fiber = Fiber.schedule do
199+
begin
200+
mutex.synchronize do
201+
condition_variable.wait(mutex)
202+
end
203+
rescue => error
204+
end
205+
end
206+
207+
fiber.raise(RuntimeError)
208+
end
209+
210+
thread.join
211+
assert_kind_of RuntimeError, error
212+
end
185213
end

thread_sync.c

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -548,49 +548,55 @@ rb_mutex_abandon_all(rb_mutex_t *mutexes)
548548
}
549549
#endif
550550

551-
static VALUE
552-
rb_mutex_sleep_forever(VALUE self)
553-
{
554-
rb_thread_sleep_deadly_allow_spurious_wakeup(self, Qnil, 0);
555-
return Qnil;
556-
}
551+
struct rb_mutex_sleep_arguments {
552+
VALUE self;
553+
VALUE timeout;
554+
};
557555

558556
static VALUE
559-
rb_mutex_wait_for(VALUE time)
560-
{
561-
rb_hrtime_t *rel = (rb_hrtime_t *)time;
562-
/* permit spurious check */
563-
return RBOOL(sleep_hrtime(GET_THREAD(), *rel, 0));
564-
}
565-
566-
VALUE
567-
rb_mutex_sleep(VALUE self, VALUE timeout)
557+
mutex_sleep_begin(VALUE _arguments)
568558
{
569-
struct timeval t;
559+
struct rb_mutex_sleep_arguments *arguments = (struct rb_mutex_sleep_arguments *)_arguments;
560+
VALUE timeout = arguments->timeout;
570561
VALUE woken = Qtrue;
571562

572-
if (!NIL_P(timeout)) {
573-
t = rb_time_interval(timeout);
574-
}
575-
576-
rb_mutex_unlock(self);
577-
time_t beg = time(0);
578-
579563
VALUE scheduler = rb_fiber_scheduler_current();
580564
if (scheduler != Qnil) {
581565
rb_fiber_scheduler_kernel_sleep(scheduler, timeout);
582-
mutex_lock_uninterruptible(self);
583566
}
584567
else {
585568
if (NIL_P(timeout)) {
586-
rb_ensure(rb_mutex_sleep_forever, self, mutex_lock_uninterruptible, self);
569+
rb_thread_sleep_deadly_allow_spurious_wakeup(arguments->self, Qnil, 0);
587570
}
588571
else {
589-
rb_hrtime_t rel = rb_timeval2hrtime(&t);
590-
woken = rb_ensure(rb_mutex_wait_for, (VALUE)&rel, mutex_lock_uninterruptible, self);
572+
struct timeval timeout_value = rb_time_interval(timeout);
573+
rb_hrtime_t relative_timeout = rb_timeval2hrtime(&timeout_value);
574+
/* permit spurious check */
575+
woken = RBOOL(sleep_hrtime(GET_THREAD(), relative_timeout, 0));
591576
}
592577
}
593578

579+
return woken;
580+
}
581+
582+
VALUE
583+
rb_mutex_sleep(VALUE self, VALUE timeout)
584+
{
585+
if (!NIL_P(timeout)) {
586+
// Validate the argument:
587+
rb_time_interval(timeout);
588+
}
589+
590+
rb_mutex_unlock(self);
591+
time_t beg = time(0);
592+
593+
struct rb_mutex_sleep_arguments arguments = {
594+
.self = self,
595+
.timeout = timeout,
596+
};
597+
598+
VALUE woken = rb_ensure(mutex_sleep_begin, (VALUE)&arguments, mutex_lock_uninterruptible, self);
599+
594600
RUBY_VM_CHECK_INTS_BLOCKING(GET_EC());
595601
if (!woken) return Qnil;
596602
time_t end = time(0) - beg;

0 commit comments

Comments
 (0)