From 6505c77501f1924571b2fe620c5c7b31ede0cd22 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Wed, 28 Jul 2021 11:05:36 +0900 Subject: [PATCH] Revert "Fix potential hang when joining threads." This reverts commit 13f8521c630a15c87398dee0763e95f59c032a94. http://rubyci.s3.amazonaws.com/solaris11-gcc/ruby-master/log/20210727T230009Z.fail.html.gz http://rubyci.s3.amazonaws.com/solaris11-sunc/ruby-master/log/20210728T000009Z.fail.html.gz This revert is to confirm whether the commit is the cause. If the failures consistently occur after this revert, I'll reintroduce the commit. --- test/fiber/scheduler.rb | 16 ++-------------- test/fiber/test_thread.rb | 14 -------------- thread.c | 21 ++++----------------- vm.c | 17 +++++++---------- 4 files changed, 13 insertions(+), 55 deletions(-) diff --git a/test/fiber/scheduler.rb b/test/fiber/scheduler.rb index 2785561000..af64e4ebb6 100644 --- a/test/fiber/scheduler.rb +++ b/test/fiber/scheduler.rb @@ -112,10 +112,8 @@ class Scheduler self.run ensure - if @urgent - @urgent.each(&:close) - @urgent = nil - end + @urgent.each(&:close) + @urgent = nil @closed = true @@ -242,13 +240,3 @@ 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 acc8300817..843604b5f1 100644 --- a/test/fiber/test_thread.rb +++ b/test/fiber/test_thread.rb @@ -66,18 +66,4 @@ 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 1232129935..3c3f645dbd 100644 --- a/thread.c +++ b/thread.c @@ -632,7 +632,6 @@ 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; @@ -818,9 +817,6 @@ 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) { @@ -861,12 +857,6 @@ 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); @@ -884,6 +874,9 @@ 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); @@ -1160,12 +1153,6 @@ 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) { @@ -1192,7 +1179,7 @@ thread_join_sleep(VALUE arg) end = rb_hrtime_add(*limit, rb_hrtime_now()); } - while (!thread_finished(target_th)) { + while (target_th->status != THREAD_KILLED) { VALUE scheduler = rb_fiber_scheduler_current(); if (scheduler != Qnil) { diff --git a/vm.c b/vm.c index 4e458b344d..accd12644e 100644 --- a/vm.c +++ b/vm.c @@ -3081,8 +3081,6 @@ 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 @@ -3095,17 +3093,16 @@ static VALUE ruby_thread_init(VALUE self) { rb_thread_t *th = GET_THREAD(); - rb_thread_t *target_th = rb_thread_ptr(self); + rb_thread_t *targe_th = rb_thread_ptr(self); rb_vm_t *vm = th->vm; - 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->vm = vm; + th_init(targe_th, self); + 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; }