kosaki	2012-11-30 22:52:34 +0900 (Fri, 30 Nov 2012)

  New Revision: 38063

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

  Log:
    * vm_core.h (rb_vm_struct): add thread_destruct_lock field.
    * thread.c (Init_Thread): ditto.
    * thread.c (rb_vm_gvl_destroy): ditto.
    
    * thread.c (thread_start_func_2): make sure vm->running_thread
      don't point to dead thread.
    * thread.c (timer_thread_function): close a race against thead
      destruction. [Bug #4911][ruby-dev:43859]
    
    * vm_core.h (rb_thread_set_current): reset running time of
      current thread instead of previous thread. We no longer
      assume previous running thread still live.

  Modified files:
    trunk/ChangeLog
    trunk/thread.c
    trunk/vm_core.h

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 38062)
+++ ChangeLog	(revision 38063)
@@ -1,3 +1,18 @@
+Fri Nov 30 22:44:14 2012  KOSAKI Motohiro  <kosaki.motohiro@g...>
+
+	* vm_core.h (rb_vm_struct): add thread_destruct_lock field.
+	* thread.c (Init_Thread): ditto.
+	* thread.c (rb_vm_gvl_destroy): ditto.
+
+	* thread.c (thread_start_func_2): make sure vm->running_thread
+	  don't point to dead thread.
+	* thread.c (timer_thread_function): close a race against thead
+	  destruction. [Bug #4911][ruby-dev:43859]
+
+	* vm_core.h (rb_thread_set_current): reset running time of
+	  current thread instead of previous thread. We no longer
+	  assume previous running thread still live.
+
 Fri Nov 30 21:57:43 2012  NAKAMURA Usaku  <usa@r...>
 
 	* revert r38053 because it causes too many test failures.
Index: vm_core.h
===================================================================
--- vm_core.h	(revision 38062)
+++ vm_core.h	(revision 38063)
@@ -333,6 +333,7 @@
     VALUE self;
 
     rb_global_vm_lock_t gvl;
+    rb_thread_lock_t    thread_destruct_lock;
 
     struct rb_thread_struct *main_thread;
     struct rb_thread_struct *running_thread;
@@ -850,7 +851,7 @@
 #define rb_thread_set_current_raw(th) (void)(ruby_current_thread = (th))
 #define rb_thread_set_current(th) do { \
     if ((th)->vm->running_thread != (th)) { \
-	(th)->vm->running_thread->running_time_us = 0; \
+	(th)->running_time_us = 0; \
     } \
     rb_thread_set_current_raw(th); \
     (th)->vm->running_thread = (th); \
Index: thread.c
===================================================================
--- thread.c	(revision 38062)
+++ thread.c	(revision 38063)
@@ -238,6 +238,7 @@
 {
     gvl_release(vm);
     gvl_destroy(vm);
+    native_mutex_destroy(&vm->thread_destruct_lock);
 }
 
 void
@@ -552,6 +553,10 @@
 	    th->stack = 0;
 	}
     }
+    native_mutex_lock(&th->vm->thread_destruct_lock);
+    /* make sure vm->running_thread never point me after this point.*/
+    th->vm->running_thread = NULL;
+    native_mutex_unlock(&th->vm->thread_destruct_lock);
     thread_cleanup_func(th, FALSE);
     gvl_release(th->vm);
 
@@ -3632,8 +3637,16 @@
 {
     rb_vm_t *vm = GET_VM(); /* TODO: fix me for Multi-VM */
 
+    /*
+     * Tricky: thread_destruct_lock doesn't close a race against
+     * vm->running_thread switch. however it guarantee th->running_thread
+     * point to valid pointer or NULL.
+     */
+    native_mutex_lock(&vm->thread_destruct_lock);
     /* for time slice */
-    RUBY_VM_SET_TIMER_INTERRUPT(vm->running_thread);
+    if (vm->running_thread)
+	RUBY_VM_SET_TIMER_INTERRUPT(vm->running_thread);
+    native_mutex_unlock(&vm->thread_destruct_lock);
 
     /* check signal */
     rb_threadptr_check_signal(vm->main_thread);
@@ -4870,6 +4883,7 @@
 	    /* acquire global vm lock */
 	    gvl_init(th->vm);
 	    gvl_acquire(th->vm, th);
+	    native_mutex_initialize(&th->vm->thread_destruct_lock);
 	    native_mutex_initialize(&th->interrupt_lock);
 
 	    th->async_errinfo_queue = rb_ary_tmp_new(0);

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/