Skip to content

Commit

Permalink
Revert "Fix potential hang when joining threads."
Browse files Browse the repository at this point in the history
This reverts commit 13f8521.

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.
  • Loading branch information
mame committed Jul 28, 2021
1 parent 4fc9ddd commit 6505c77
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 55 deletions.
16 changes: 2 additions & 14 deletions test/fiber/scheduler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,8 @@ def close

self.run
ensure
if @urgent
@urgent.each(&:close)
@urgent = nil
end
@urgent.each(&:close)
@urgent = nil

@closed = true

Expand Down Expand Up @@ -242,13 +240,3 @@ def unblock(blocker, fiber)
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
14 changes: 0 additions & 14 deletions test/fiber/test_thread.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,4 @@ def test_broken_unblock
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
21 changes: 4 additions & 17 deletions thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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)
{
Expand All @@ -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) {
Expand Down
17 changes: 7 additions & 10 deletions vm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}

Expand Down

0 comments on commit 6505c77

Please sign in to comment.