8000 Handle spurious wakeups in `Thread#join`. · ruby/ruby@3648725 · GitHub
[go: up one dir, main page]
Skip to content

Commit 3648725

Browse files
samuel-williams-shopifysamuel-williams-shopify
committed
Handle spurious wakeups in Thread#join.
1 parent 4e39580 commit 3648725

File tree

3 files changed

+59
-12
lines changed

3 files changed

+59
-12
lines changed

‎test/fiber/scheduler.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ def run
126126
end
127127

128128
ready.each do |fiber|
129-
fiber.transfer
129+
fiber.transfer if fiber.alive?
130130
end
131131
end
132132
end

‎test/fiber/test_thread.rb

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,47 @@ def test_thread_join_blocking
9090
assert_equal :done, thread.value
9191
end
9292

93+
def test_spurious_unblock_during_thread_join
94+
ready = Thread::Queue.new
95+
96+
target_thread = Thread.new do
97+
ready.pop
98+
:success
99+
end
100+
101+
Thread.pass until target_thread.status == "sleep"
102+
103+
result = nil
104+
105+
thread = Thread.new do
106+
scheduler = Scheduler.new
107+
Fiber.set_scheduler scheduler
108+
109+
# Create a fiber that will join a long-running thread:
110+
joining_fiber = Fiber.schedule do
111+
result = target_thread.value
112+
end
113+
114+
# Create another fiber that spuriously unblocks the joining fiber:
115+
Fiber.schedule do
116+
# This interrupts the join in joining_fiber:
117+
scheduler.unblock(:spurious_wakeup, joining_fiber)
118+
119+
# This allows the unblock to be processed:
120+
sleep(0)
121+
122+
# This allows the target thread to finish:
123+
ready.push(:done)
124+
end
125+
126+
scheduler.run
127+
end
128+
129+
thread.join
130+
131+
assert_equal :success, result
132+
end
133+
93134
def test_broken_unblock
94135
thread = Thread.new do
95136
Thread.current.report_on_exception = false

‎thread.c

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,23 +1052,28 @@ thread_join_sleep(VALUE arg)
10521052
while (!thread_finished(target_th)) {
10531053
VALUE scheduler = rb_fiber_scheduler_current();
10541054

1055-
if (scheduler != Qnil) {
1056-
rb_fiber_scheduler_block(scheduler, target_th->self, p->timeout);
1057-
// Check if the target thread is finished after blocking:
1058-
if (thread_finished(target_th)) break;
1059-
// Otherwise, a timeout occurred:
1060-
else return Qfalse;
1061-
}
1062-
else if (!limit) {
1063-
sleep_forever(th, SLEEP_DEADLOCKABLE | SLEEP_ALLOW_SPURIOUS | SLEEP_NO_CHECKINTS);
1055+
if (!limit) {
1056+
if (scheduler != Qnil) {
1057+
rb_fiber_scheduler_block(scheduler, target_th->self, Qnil);
1058+
}
1059+
else {
1060+
sleep_forever(th, SLEEP_DEADLOCKABLE | SLEEP_ALLOW_SPURIOUS | SLEEP_NO_CHECKINTS);
1061+
}
10641062
}
10651063
else {
10661064
if (hrtime_update_expire(limit, end)) {
10671065
RUBY_DEBUG_LOG("timeout target_th:%u", rb_th_serial(target_th));
10681066
return Qfalse;
10691067
}
1070-
th->status = THREAD_STOPPED;
1071-
native_sleep(th, limit);
1068+
1069+
if (scheduler != Qnil) {
1070+
VALUE timeout = rb_float_new(hrtime2double(*limit));
1071+
rb_fiber_scheduler_block(scheduler, target_th->self, timeout);
1072+
}
1073+
else {
1074+
th->status = THREAD_STOPPED;
1075+
native_sleep(th, limit);
1076+
}
10721077
}
10731078
RUBY_VM_CHECK_INTS_BLOCKING(th->ec);
10741079
th->status = THREAD_RUNNABLE;
@@ -1745,6 +1750,7 @@ io_blocking_operation_exit(VALUE _arguments)
17451750
rb_fiber_t *fiber = io->closing_ec->fiber_ptr;
17461751

17471752
if (thread->scheduler != Qnil) {
1753+
// This can cause spurious wakeups...
17481754
rb_fiber_scheduler_unblock(thread->scheduler, io->self, rb_fiberptr_self(fiber));
17491755
}
17501756
else {

0 commit comments

Comments
 (0)
0