From 15b25acd2541ae37f4f874b50128d34d8b079457 Mon Sep 17 00:00:00 2001 From: kosaki Date: Fri, 30 Nov 2012 13:52:34 +0000 Subject: [PATCH] * 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. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@38063 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 15 +++++++++++++++ thread.c | 16 +++++++++++++++- vm_core.h | 3 ++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index f40bc37c76..f2f2cf164a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +Fri Nov 30 22:44:14 2012 KOSAKI Motohiro + + * 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 * revert r38053 because it causes too many test failures. diff --git a/thread.c b/thread.c index 5a97b5202f..43b1ad19a9 100644 --- a/thread.c +++ b/thread.c @@ -238,6 +238,7 @@ rb_vm_gvl_destroy(rb_vm_t *vm) { gvl_release(vm); gvl_destroy(vm); + native_mutex_destroy(&vm->thread_destruct_lock); } void @@ -552,6 +553,10 @@ thread_start_func_2(rb_thread_t *th, VALUE *stack_start, VALUE *register_stack_s 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 @@ timer_thread_function(void *arg) { 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 @@ Init_Thread(void) /* 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); diff --git a/vm_core.h b/vm_core.h index 96b55c4931..dcd6cd109e 100644 --- a/vm_core.h +++ b/vm_core.h @@ -333,6 +333,7 @@ typedef struct rb_vm_struct { 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 @@ GET_THREAD(void) #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); \