diff --git a/test/fiber/scheduler.rb b/test/fiber/scheduler.rb index af64e4ebb6..2785561000 100644 --- a/test/fiber/scheduler.rb +++ b/test/fiber/scheduler.rb @@ -112,8 +112,10 @@ class Scheduler self.run ensure - @urgent.each(&:close) - @urgent = nil + if @urgent + @urgent.each(&:close) + @urgent = nil + end @closed = true @@ -240,3 +242,13 @@ class BrokenUnblockScheduler < Scheduler raise "Broken unblock!" end end + +class SleepingUnblockScheduler < Scheduler + # This method is invoked when the thread is exiting. + def unblock(blocker, fiber) + super + + # This changes the current thread state to `THREAD_RUNNING` which causes `thread_join_sleep` to hang. + sleep(0.1) + end +end diff --git a/test/fiber/test_thread.rb b/test/fiber/test_thread.rb index 843604b5f1..acc8300817 100644 --- a/test/fiber/test_thread.rb +++ b/test/fiber/test_thread.rb @@ -66,4 +66,18 @@ class TestFiberThread < Test::Unit::TestCase thread.join end end + + def test_thread_join_hang + thread = Thread.new do + scheduler = SleepingUnblockScheduler.new + + Fiber.set_scheduler scheduler + + Fiber.schedule do + Thread.new{sleep(0.01)}.value + end + end + + thread.join + end end diff --git a/thread.c b/thread.c index 0f6838e141..ec67bba2d5 100644 --- a/thread.c +++ b/thread.c @@ -632,6 +632,7 @@ thread_cleanup_func_before_exec(void *th_ptr) { rb_thread_t *th = th_ptr; th->status = THREAD_KILLED; + // The thread stack doesn't exist in the forked process: th->ec->machine.stack_start = th->ec->machine.stack_end = NULL; @@ -817,6 +818,9 @@ thread_start_func_2(rb_thread_t *th, VALUE *stack_start) thread_debug("thread start (get lock): %p\n", (void *)th); + // Ensure that we are not joinable. + VM_ASSERT(th->value == Qundef); + EC_PUSH_TAG(th->ec); if ((state = EC_EXEC_TAG()) == TAG_NONE) { @@ -857,6 +861,12 @@ thread_start_func_2(rb_thread_t *th, VALUE *stack_start) th->value = Qnil; } + // The thread is effectively finished and can be joined. + VM_ASSERT(th->value != Qundef); + + rb_threadptr_join_list_wakeup(th); + rb_threadptr_unlock_all_locking_mutexes(th); + if (th->invoke_type == thread_invoke_type_ractor_proc) { rb_thread_terminate_all(th); rb_ractor_teardown(th->ec); @@ -874,9 +884,6 @@ thread_start_func_2(rb_thread_t *th, VALUE *stack_start) rb_threadptr_raise(ractor_main_th, 1, &errinfo); } - rb_threadptr_join_list_wakeup(th); - rb_threadptr_unlock_all_locking_mutexes(th); - EC_POP_TAG(); rb_ec_clear_current_thread_trace_func(th->ec); @@ -1153,6 +1160,12 @@ remove_from_join_list(VALUE arg) static rb_hrtime_t *double2hrtime(rb_hrtime_t *, double); +static int +thread_finished(rb_thread_t *th) +{ + return th->status == THREAD_KILLED || th->value != Qundef; +} + static VALUE thread_join_sleep(VALUE arg) { @@ -1179,7 +1192,7 @@ thread_join_sleep(VALUE arg) end = rb_hrtime_add(*limit, rb_hrtime_now()); } - while (target_th->status != THREAD_KILLED) { + while (!thread_finished(target_th)) { VALUE scheduler = rb_fiber_scheduler_current(); if (scheduler != Qnil) { diff --git a/vm.c b/vm.c index 78aadb6a41..75cd371c97 100644 --- a/vm.c +++ b/vm.c @@ -3081,6 +3081,8 @@ th_init(rb_thread_t *th, VALUE self) th->thread_id_string[0] = '\0'; #endif + th->value = Qundef; + #if OPT_CALL_THREADED_CODE th->retval = Qundef; #endif @@ -3093,16 +3095,17 @@ static VALUE ruby_thread_init(VALUE self) { rb_thread_t *th = GET_THREAD(); - rb_thread_t *targe_th = rb_thread_ptr(self); + rb_thread_t *target_th = rb_thread_ptr(self); rb_vm_t *vm = th->vm; - targe_th->vm = vm; - th_init(targe_th, self); + target_th->vm = vm; + th_init(target_th, self); + + target_th->top_wrapper = 0; + target_th->top_self = rb_vm_top_self(); + target_th->ec->root_svar = Qfalse; + target_th->ractor = th->ractor; - targe_th->top_wrapper = 0; - targe_th->top_self = rb_vm_top_self(); - targe_th->ec->root_svar = Qfalse; - targe_th->ractor = th->ractor; return self; }