Handle spurious wakeups in Thread#join. (#13532)

Merged-By: ioquatix <samuel@codeotaku.com>

test/fiber/scheduler.rb

@@ -126,7 +126,7 @@ class Scheduler
         end
 
         ready.each do |fiber|
-          fiber.transfer
+          fiber.transfer if fiber.alive?
         end
       end
     end

test/fiber/test_thread.rb

@@ -90,6 +90,47 @@ class TestFiberThread < Test::Unit::TestCase
     assert_equal :done, thread.value
   end
 
+  def test_spurious_unblock_during_thread_join
+    ready = Thread::Queue.new
+
+    target_thread = Thread.new do
+      ready.pop
+      :success
+    end
+
+    Thread.pass until target_thread.status == "sleep"
+
+    result = nil
+
+    thread = Thread.new do
+      scheduler = Scheduler.new
+      Fiber.set_scheduler scheduler
+
+      # Create a fiber that will join a long-running thread:
+      joining_fiber = Fiber.schedule do
+        result = target_thread.value
+      end
+
+      # Create another fiber that spuriously unblocks the joining fiber:
+      Fiber.schedule do
+        # This interrupts the join in joining_fiber:
+        scheduler.unblock(:spurious_wakeup, joining_fiber)
+
+        # This allows the unblock to be processed:
+        sleep(0)
+
+        # This allows the target thread to finish:
+        ready.push(:done)
+      end
+
+      scheduler.run
+    end
+
+    thread.join
+
+    assert_equal :success, result
+  end
+
   def test_broken_unblock
     thread = Thread.new do
       Thread.current.report_on_exception = false

thread.c

@@ -1052,23 +1052,28 @@ thread_join_sleep(VALUE arg)
     while (!thread_finished(target_th)) {
         VALUE scheduler = rb_fiber_scheduler_current();
 
-        if (scheduler != Qnil) {
+        if (!limit) {
-            rb_fiber_scheduler_block(scheduler, target_th->self, p->timeout);
+            if (scheduler != Qnil) {
-            // Check if the target thread is finished after blocking:
+                rb_fiber_scheduler_block(scheduler, target_th->self, Qnil);
-            if (thread_finished(target_th)) break;
+            }
-            // Otherwise, a timeout occurred:
+            else {
-            else return Qfalse;
+                sleep_forever(th, SLEEP_DEADLOCKABLE | SLEEP_ALLOW_SPURIOUS | SLEEP_NO_CHECKINTS);
-        }
+            }
-        else if (!limit) {
-            sleep_forever(th, SLEEP_DEADLOCKABLE | SLEEP_ALLOW_SPURIOUS | SLEEP_NO_CHECKINTS);
         }
         else {
             if (hrtime_update_expire(limit, end)) {
                 RUBY_DEBUG_LOG("timeout target_th:%u", rb_th_serial(target_th));
                 return Qfalse;
             }
-            th->status = THREAD_STOPPED;
+
-            native_sleep(th, limit);
+            if (scheduler != Qnil) {
+                VALUE timeout = rb_float_new(hrtime2double(*limit));
+                rb_fiber_scheduler_block(scheduler, target_th->self, timeout);
+            }
+            else {
+                th->status = THREAD_STOPPED;
+                native_sleep(th, limit);
+            }
         }
         RUBY_VM_CHECK_INTS_BLOCKING(th->ec);
         th->status = THREAD_RUNNABLE;
@@ -1749,6 +1754,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 {