* 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

@@ -1,3 +1,30 @@
+Wed Nov 28 16:40:14 2012  KOSAKI Motohiro  <kosaki.motohiro@gmail.com>
+
+	* 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  <kosaki.motohiro@gmail.com>
 
 	* thread.c (struct rb_mutex_struct): add allow_trap field.

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(/^#<TestThread::Thread:.* dead>$/, 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

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);
 

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) {
+    if (th->to_kill || th->status == THREAD_KILLED)
-      case THREAD_RUNNABLE:
-      case THREAD_STOPPED:
-      case THREAD_STOPPED_FOREVER:
-	break;
-      case THREAD_TO_KILL:
-      case THREAD_KILLED:
 	return Qnil;
-    }
 
     return vm_backtrace_to_ary(th, argc, argv, 0, 0, to_str);
 }

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;