From 9cbf473287b6e0fca089c30db1cd276b3c2d477c Mon Sep 17 00:00:00 2001 From: kosaki Date: Wed, 28 Nov 2012 08:31:03 +0000 Subject: * vm_core.h (enum rb_thread_status): remove THREAD_TO_KILL * vm_core.h (struct rb_thread_struct): add to_kill field * thread.c (terminate_i): convert THREAD_TO_KILL to to_kill. * thread.c (rb_threadptr_to_kill): ditto. * thread.c (rb_thread_kill): ditto. * thread.c (rb_thread_wakeup_alive): ditto. * thread.c (thread_list_i): ditto. * thread.c (static const char): ditto. * thread.c (thread_status_name): ditto. * thread.c (rb_thread_status): ditto. * thread.c (rb_thread_inspect): ditto. * vm_backtrace.c (thread_backtrace_to_ary): ditto. * thread.c (rb_threadptr_execute_interrupts): fix thread status overwritten issue. [Bug #7450] [ruby-core:50249] * test/ruby/test_thread.rb (test_hread_status_raise_after_kill): test for the above. * test/ruby/test_thread.rb (test_thread_status_in_trap): test for thread status in trap. * test/ruby/test_thread.rb (test_status_and_stop_p): remove Thread.control_interrupt unsafe test. Thread#kill no longer changes thread status. Instead of, Thread#kill receiver changes their own status when receiving kill signal. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@37931 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 27 +++++++++++++++++++++++++ test/ruby/test_thread.rb | 51 ++++++++++++++++++++++++++++++++++++++++-------- thread.c | 39 +++++++++++++++++++----------------- vm_backtrace.c | 9 +-------- vm_core.h | 2 +- 5 files changed, 93 insertions(+), 35 deletions(-) diff --git a/ChangeLog b/ChangeLog index b26e2c4368..6227999184 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,30 @@ +Wed Nov 28 16:40:14 2012 KOSAKI Motohiro + + * vm_core.h (enum rb_thread_status): remove THREAD_TO_KILL + * vm_core.h (struct rb_thread_struct): add to_kill field + * thread.c (terminate_i): convert THREAD_TO_KILL to to_kill. + * thread.c (rb_threadptr_to_kill): ditto. + * thread.c (rb_thread_kill): ditto. + * thread.c (rb_thread_wakeup_alive): ditto. + * thread.c (thread_list_i): ditto. + * thread.c (static const char): ditto. + * thread.c (thread_status_name): ditto. + * thread.c (rb_thread_status): ditto. + * thread.c (rb_thread_inspect): ditto. + * vm_backtrace.c (thread_backtrace_to_ary): ditto. + + * thread.c (rb_threadptr_execute_interrupts): fix thread status + overwritten issue. [Bug #7450] [ruby-core:50249] + + * test/ruby/test_thread.rb (test_hread_status_raise_after_kill): + test for the above. + * test/ruby/test_thread.rb (test_thread_status_in_trap): test for + thread status in trap. + * test/ruby/test_thread.rb (test_status_and_stop_p): remove + Thread.control_interrupt unsafe test. Thread#kill no longer + changes thread status. Instead of, Thread#kill receiver changes + their own status when receiving kill signal. + Wed Nov 28 16:21:46 2012 KOSAKI Motohiro * thread.c (struct rb_mutex_struct): add allow_trap field. diff --git a/test/ruby/test_thread.rb b/test/ruby/test_thread.rb index 30e886447e..45f389b0fa 100644 --- a/test/ruby/test_thread.rb +++ b/test/ruby/test_thread.rb @@ -497,7 +497,6 @@ class TestThread < Test::Unit::TestCase a = ::Thread.new { raise("die now") } b = Thread.new { Thread.stop } c = Thread.new { Thread.exit } - d = Thread.new { sleep } e = Thread.current sleep 0.5 @@ -511,21 +510,14 @@ class TestThread < Test::Unit::TestCase assert_match(/^#$/, c.inspect) assert(c.stop?) - d.kill - # to avoid thread switching... - ds1 = d.status - ds2 = d.stop? es1 = e.status es2 = e.stop? - assert_equal(["aborting", false], [ds1, ds2]) - assert_equal(["run", false], [es1, es2]) ensure a.kill if a b.kill if b c.kill if c - d.kill if d end def test_safe_level @@ -912,4 +904,47 @@ Thread.new(Thread.current) {|mth| } INPUT end + + def test_thread_status_in_trap + # when running trap handler, Thread#status must show "run" + # Even though interrupted from sleeping function + assert_in_out_err([], <<-INPUT, %w(sleep run), []) + Signal.trap(:INT) { + puts Thread.current.status + } + + Thread.new(Thread.current) {|mth| + sleep 0.01 + puts mth.status + Process.kill(:INT, $$) + } + sleep 0.1 + INPUT + end + + # Bug #7450 + def test_thread_status_raise_after_kill + ary = [] + + t = Thread.new { + begin + ary << Thread.current.status + sleep + ensure + begin + ary << Thread.current.status + sleep + ensure + ary << Thread.current.status + end + end + } + + sleep 0.01 + t.kill + sleep 0.01 + t.raise + sleep 0.01 + assert_equal(ary, ["run", "aborting", "aborting"]) + end end diff --git a/thread.c b/thread.c index f32da2c282..af33c920f6 100644 --- a/thread.c +++ b/thread.c @@ -326,7 +326,6 @@ terminate_i(st_data_t key, st_data_t val, rb_thread_t *main_thread) if (th != main_thread) { thread_debug("terminate_i: %p\n", (void *)th); rb_threadptr_async_errinfo_enque(th, eTerminateSignal); - th->status = THREAD_TO_KILL; rb_threadptr_interrupt(th); } else { @@ -1732,7 +1731,8 @@ static void rb_threadptr_to_kill(rb_thread_t *th) { rb_threadptr_async_errinfo_clear(th); - th->status = THREAD_TO_KILL; + th->status = THREAD_RUNNABLE; + th->to_kill = 1; th->errinfo = INT2FIX(TAG_FATAL); TH_JUMP_TAG(th, TAG_FATAL); } @@ -1743,7 +1743,6 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing) if (th->raised_flag) return; while (1) { - enum rb_thread_status status = th->status; rb_atomic_t interrupt; rb_atomic_t old; int sig; @@ -1766,13 +1765,16 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing) finalizer_interrupt = interrupt & FINALIZER_INTERRUPT_MASK; trap_interrupt = interrupt & TRAP_INTERRUPT_MASK; - th->status = THREAD_RUNNABLE; + /* signal handling */ if (trap_interrupt && (th == th->vm->main_thread)) { + enum rb_thread_status prev_status = th->status; + th->status = THREAD_RUNNABLE; while ((sig = rb_get_next_signal()) != 0) { rb_signal_exec(th, sig); } + th->status = prev_status; } /* exception from another thread */ @@ -1788,10 +1790,13 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing) rb_threadptr_to_kill(th); } else { + /* set runnable if th was slept. */ + if (th->status == THREAD_STOPPED || + th->status == THREAD_STOPPED_FOREVER) + th->status = THREAD_RUNNABLE; rb_exc_raise(err); } } - th->status = status; if (finalizer_interrupt) { rb_gc_finalize_deferred(); @@ -1805,7 +1810,7 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing) else limits_us >>= -th->priority; - if (status == THREAD_RUNNABLE || status == THREAD_TO_KILL) + if (th->status == THREAD_RUNNABLE) th->running_time_us += TIME_QUANTUM_USEC; EXEC_EVENT_HOOK(th, RUBY_EVENT_SWITCH, th->cfp->self, 0, 0, Qundef); @@ -1985,7 +1990,7 @@ rb_thread_kill(VALUE thread) if (th != GET_THREAD() && th->safe_level < 4) { rb_secure(4); } - if (th->status == THREAD_TO_KILL || th->status == THREAD_KILLED) { + if (th->to_kill || th->status == THREAD_KILLED) { return thread; } if (th == th->vm->main_thread) { @@ -2000,7 +2005,6 @@ rb_thread_kill(VALUE thread) } else { rb_threadptr_async_errinfo_enque(th, eKillSignal); - th->status = THREAD_TO_KILL; rb_threadptr_interrupt(th); } return thread; @@ -2082,9 +2086,8 @@ rb_thread_wakeup_alive(VALUE thread) return Qnil; } rb_threadptr_ready(th); - if (th->status != THREAD_TO_KILL) { + if (th->status == THREAD_STOPPED || th->status == THREAD_STOPPED_FOREVER) th->status = THREAD_RUNNABLE; - } return thread; } @@ -2157,7 +2160,6 @@ thread_list_i(st_data_t key, st_data_t val, void *data) case THREAD_RUNNABLE: case THREAD_STOPPED: case THREAD_STOPPED_FOREVER: - case THREAD_TO_KILL: rb_ary_push(ary, th->self); default: break; @@ -2352,16 +2354,17 @@ rb_thread_group(VALUE thread) } static const char * -thread_status_name(enum rb_thread_status status) +thread_status_name(rb_thread_t *th) { - switch (status) { + switch (th->status) { case THREAD_RUNNABLE: - return "run"; + if (th->to_kill) + return "aborting"; + else + return "run"; case THREAD_STOPPED: case THREAD_STOPPED_FOREVER: return "sleep"; - case THREAD_TO_KILL: - return "aborting"; case THREAD_KILLED: return "dead"; default: @@ -2411,7 +2414,7 @@ rb_thread_status(VALUE thread) } return Qfalse; } - return rb_str_new2(thread_status_name(th->status)); + return rb_str_new2(thread_status_name(th)); } @@ -2500,7 +2503,7 @@ rb_thread_inspect(VALUE thread) VALUE str; GetThreadPtr(thread, th); - status = thread_status_name(th->status); + status = thread_status_name(th); str = rb_sprintf("#<%s:%p %s>", cname, (void *)thread, status); OBJ_INFECT(str, thread); diff --git a/vm_backtrace.c b/vm_backtrace.c index 6696391aba..595edc65bf 100644 --- a/vm_backtrace.c +++ b/vm_backtrace.c @@ -738,15 +738,8 @@ thread_backtrace_to_ary(int argc, VALUE *argv, VALUE thval, int to_str) rb_thread_t *th; GetThreadPtr(thval, th); - switch (th->status) { - case THREAD_RUNNABLE: - case THREAD_STOPPED: - case THREAD_STOPPED_FOREVER: - break; - case THREAD_TO_KILL: - case THREAD_KILLED: + if (th->to_kill || th->status == THREAD_KILLED) return Qnil; - } return vm_backtrace_to_ary(th, argc, argv, 0, 0, to_str); } diff --git a/vm_core.h b/vm_core.h index a3059e8c53..bc8c93bbea 100644 --- a/vm_core.h +++ b/vm_core.h @@ -425,7 +425,6 @@ extern const rb_data_type_t ruby_threadptr_data_type; TypedData_Get_Struct((obj), rb_thread_t, &ruby_threadptr_data_type, (ptr)) enum rb_thread_status { - THREAD_TO_KILL, THREAD_RUNNABLE, THREAD_STOPPED, THREAD_STOPPED_FOREVER, @@ -498,6 +497,7 @@ typedef struct rb_thread_struct { /* thread control */ rb_thread_id_t thread_id; enum rb_thread_status status; + int to_kill; int priority; native_thread_data_t native_thread_data; -- cgit v1.2.3