[ruby-dev:46652] Fwd: [ruby-changes:25874] kosaki:r37931 (trunk): * vm_core.h (enum rb_thread_status): remove THREAD_TO_KILL

THREAD_TO_KILL を消す理由とかを記録に残して頂くと良かったかと．

そもそも THREAD_TO_KILL がうまく機能してないから一度整理したぜ，って感じ
でしょうか．

-------- Original Message --------
Subject: [ruby-changes:25874] kosaki:r37931 (trunk): * vm_core.h (enum
rb_thread_status): remove THREAD_TO_KILL
Date: Wed, 28 Nov 2012 17:31:12 +0900 (JST)
From: kosaki <[e-mail]>
Reply-To: [e-mail]
To: [e-mail]

kosaki	2012-11-28 17:31:03 +0900 (Wed, 28 Nov 2012)

  New Revision: 37931

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=37931

  Log:
    * 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.

  Modified files:
    trunk/ChangeLog
    trunk/test/ruby/test_thread.rb
    trunk/thread.c
    trunk/vm_backtrace.c
    trunk/vm_core.h

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 37930)
+++ ChangeLog	(revision 37931)
@@ -1,3 +1,30 @@
+Wed Nov 28 16:40:14 2012  KOSAKI Motohiro  <[e-mail]>
+
+	* 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  <[e-mail]>

 	* thread.c (struct rb_mutex_struct): add allow_trap field.
Index: vm_core.h
===================================================================
--- vm_core.h	(revision 37930)
+++ vm_core.h	(revision 37931)
@@ -425,7 +425,6 @@
     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 @@
     /* 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;
Index: thread.c
===================================================================
--- thread.c	(revision 37930)
+++ thread.c	(revision 37931)
@@ -326,7 +326,6 @@
     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 @@
 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 @@
     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 @@
 	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_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 @@
 	    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 @@
     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 @@
     }
     else {
 	rb_threadptr_async_errinfo_enque(th, eKillSignal);
-	th->status = THREAD_TO_KILL;
 	rb_threadptr_interrupt(th);
     }
     return thread;
@@ -2082,9 +2086,8 @@
 	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 @@
       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 @@
 }

 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 @@
 	}
 	return Qfalse;
     }
-    return rb_str_new2(thread_status_name(th->status));
+    return rb_str_new2(thread_status_name(th));
 }


@@ -2500,7 +2503,7 @@
     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);

Index: vm_backtrace.c
===================================================================
--- vm_backtrace.c	(revision 37930)
+++ vm_backtrace.c	(revision 37931)
@@ -738,15 +738,8 @@
     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);
 }
Index: test/ruby/test_thread.rb
===================================================================
--- test/ruby/test_thread.rb	(revision 37930)
+++ test/ruby/test_thread.rb	(revision 37931)
@@ -497,7 +497,6 @@
     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 @@
     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 @@
 }
     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

--
ML: [e-mail]
Info: http://www.atdot.net/~ko1/quickml/