Handle spurious wakeups in Thread#join.

samuel-williams-shopify · samuel-williams-shopify · commit 70206369b4b5 · 2025-06-05T19:29:17.000+09:00
diff --git a/scheduler.c b/scheduler.c
@@ -146,18 +146,18 @@ blocking_operation_call(VALUE self)
     rb_fiber_scheduler_blocking_operation_t *blocking_operation = get_blocking_operation(self);
 
     if (blocking_operation->status != RB_FIBER_SCHEDULER_BLOCKING_OPERATION_STATUS_QUEUED) {
-        rb_raise(rb_eRuntimeError, "Blocking operation has already been executed");
+        rb_raise(rb_eRuntimeError, "Blocking operation has already been executed!");
     }
 
     if (blocking_operation->function == NULL) {
-        rb_raise(rb_eRuntimeError, "Blocking operation has no function to execute");
+        rb_raise(rb_eRuntimeError, "Blocking operation has no function to execute!");
     }
 
     if (blocking_operation->state == NULL) {
-        rb_raise(rb_eRuntimeError, "Blocking operation has no result object");
+        rb_raise(rb_eRuntimeError, "Blocking operation has no result object!");
     }
 
-        // Mark as executing
+    // Mark as executing
     blocking_operation->status = RB_FIBER_SCHEDULER_BLOCKING_OPERATION_STATUS_EXECUTING;
 
     // Execute the blocking operation without GVL
@@ -1076,7 +1076,7 @@ VALUE rb_fiber_scheduler_blocking_operation_wait(VALUE scheduler, void* (*functi
 
     // If the blocking operation was never executed, return Qundef to signal
     // the caller to use rb_nogvl instead
-    if (current_status == RB_FIBER_SCHEDULER_BLOCKING_OPERATION_STATUS_QUEUED) {
+    if (current_status != RB_FIBER_SCHEDULER_BLOCKING_OPERATION_STATUS_COMPLETED) {
         return Qundef;
     }
 
diff --git a/thread.c b/thread.c
@@ -1060,8 +1060,10 @@ thread_join_sleep(VALUE arg)
             rb_fiber_scheduler_block(scheduler, target_th->self, p->timeout);
             // Check if the target thread is finished after blocking:
             if (thread_finished(target_th)) break;
-            // Otherwise, a timeout occurred:
-            else return Qfalse;
+            // Otherwise, a timeout may have occurred:
+            else if (RTEST(p->timeout)) {
+                return Qfalse;
+            }
         }
         else if (!limit) {
             sleep_forever(th, SLEEP_DEADLOCKABLE | SLEEP_ALLOW_SPURIOUS | SLEEP_NO_CHECKINTS);
@@ -1749,6 +1751,7 @@ io_blocking_operation_exit(VALUE _arguments)
     rb_fiber_t *fiber = io->closing_ec->fiber_ptr;
 
     if (thread->scheduler != Qnil) {
+        // This can cause spurious wakeups...
         rb_fiber_scheduler_unblock(thread->scheduler, io->self, rb_fiberptr_self(fiber));
     }
     else {

Original file line number	Diff line number	Diff line change
`@@ -146,18 +146,18 @@ blocking_operation_call(VALUE self)`
`146`	`146`	`rb_fiber_scheduler_blocking_operation_t *blocking_operation = get_blocking_operation(self);`
`147`	`147`
`148`	`148`	`if (blocking_operation->status != RB_FIBER_SCHEDULER_BLOCKING_OPERATION_STATUS_QUEUED) {`
`149`		`- rb_raise(rb_eRuntimeError, "Blocking operation has already been executed");`
	`149`	`+ rb_raise(rb_eRuntimeError, "Blocking operation has already been executed!");`
`150`	`150`	`}`
`151`	`151`
`152`	`152`	`if (blocking_operation->function == NULL) {`
`153`		`- rb_raise(rb_eRuntimeError, "Blocking operation has no function to execute");`
	`153`	`+ rb_raise(rb_eRuntimeError, "Blocking operation has no function to execute!");`
`154`	`154`	`}`
`155`	`155`
`156`	`156`	`if (blocking_operation->state == NULL) {`
`157`		`- rb_raise(rb_eRuntimeError, "Blocking operation has no result object");`
	`157`	`+ rb_raise(rb_eRuntimeError, "Blocking operation has no result object!");`
`158`	`158`	`}`
`159`	`159`
`160`		`- // Mark as executing`
	`160`	`+ // Mark as executing`
`161`	`161`	`blocking_operation->status = RB_FIBER_SCHEDULER_BLOCKING_OPERATION_STATUS_EXECUTING;`
`162`	`162`
`163`	`163`	`// Execute the blocking operation without GVL`
`@@ -1076,7 +1076,7 @@ VALUE rb_fiber_scheduler_blocking_operation_wait(VALUE scheduler, void* (*functi`
`1076`	`1076`
`1077`	`1077`	`// If the blocking operation was never executed, return Qundef to signal`
`1078`	`1078`	`// the caller to use rb_nogvl instead`
`1079`		`- if (current_status == RB_FIBER_SCHEDULER_BLOCKING_OPERATION_STATUS_QUEUED) {`
	`1079`	`+ if (current_status != RB_FIBER_SCHEDULER_BLOCKING_OPERATION_STATUS_COMPLETED) {`
`1080`	`1080`	`return Qundef;`
`1081`	`1081`	`}`
`1082`	`1082`