ruby-changes:54280
From: normal <ko1@a...>
Date: Sat, 22 Dec 2018 10:41:24 +0900 (JST)
Subject: [ruby-changes:54280] normal:r66489 (trunk): thread_sync.c (rb_mutex_t): eliminate fork_gen
normal 2018-12-22 10:41:18 +0900 (Sat, 22 Dec 2018) New Revision: 66489 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=66489 Log: thread_sync.c (rb_mutex_t): eliminate fork_gen The true bug fork_gen was hiding was rb_mutex_abandon_locking_mutex failing to unconditionally clear the waitq of mutexes it was waiting on. So we fix rb_mutex_abandon_locking_mutex, instead, and eliminate rb_mutex_cleanup_keeping_mutexes. This commit was tested heavily on a single-core Pentium-M which was my most reliable reproducer of the "crash.rb" script from [Bug #15383] [Bug #14578] [Bug #15383] Note: [Bug #15430] turned out to be an entirely different problem: RLIMIT_NPROC limit was hit on the CI VMs. Modified files: trunk/thread.c trunk/thread_sync.c Index: thread_sync.c =================================================================== --- thread_sync.c (revision 66488) +++ thread_sync.c (revision 66489) @@ -45,17 +45,6 @@ typedef struct rb_mutex_struct { https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L45 rb_thread_t *th; struct rb_mutex_struct *next_mutex; struct list_head waitq; /* protected by GVL */ - - /* - * FIXME: belt-and-suspenders redundancy. This field should NOT - * be necessary at all if: - * - rb_mutex_cleanup_keeping_mutexes - * - rb_mutex_abandon_keeping_mutexes - * - rb_mutex_abandon_locking_mutex - * are all correct, but I suspect one of them is buggy. - * [Bug #15383] [Bug #15430] - */ - rb_serial_t fork_gen; } rb_mutex_t; #if defined(HAVE_WORKING_FORK) @@ -132,17 +121,9 @@ static rb_mutex_t * https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L121 mutex_ptr(VALUE obj) { rb_mutex_t *mutex; - rb_serial_t fork_gen = GET_VM()->fork_gen; TypedData_Get_Struct(obj, rb_mutex_t, &mutex_data_type, mutex); - /* FIXME: remove (see comment at rb_mutex_t definition) */ - if (mutex->fork_gen != fork_gen) { - /* forked children can't reach into parent thread stacks */ - mutex->fork_gen = fork_gen; - list_head_init(&mutex->waitq); - } - return mutex; } @@ -428,8 +409,7 @@ rb_mutex_abandon_locking_mutex(rb_thread https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L409 if (th->locking_mutex) { rb_mutex_t *mutex = mutex_ptr(th->locking_mutex); - if (mutex->th == th) - rb_mutex_abandon_all(mutex); + list_head_init(&mutex->waitq); th->locking_mutex = Qfalse; } } @@ -447,25 +427,6 @@ rb_mutex_abandon_all(rb_mutex_t *mutexes https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L427 list_head_init(&mutex->waitq); } } - -/* - * All other threads are dead in the a new child process, so waitqs - * contain references to dead threads which we need to clean up - */ -static void -rb_mutex_cleanup_keeping_mutexes(const rb_thread_t *current_thread) -{ - rb_mutex_t *mutex = current_thread->keeping_mutexes; - rb_serial_t fork_gen = current_thread->vm->fork_gen; - - while (mutex) { - /* FIXME: remove (see comment at rb_mutex_t definition) */ - mutex->fork_gen = fork_gen; - - list_head_init(&mutex->waitq); - mutex = mutex->next_mutex; - } -} #endif static VALUE Index: thread.c =================================================================== --- thread.c (revision 66488) +++ thread.c (revision 66489) @@ -4449,7 +4449,6 @@ rb_thread_atfork(void) https://github.com/ruby/ruby/blob/trunk/thread.c#L4449 rb_thread_t *th = GET_THREAD(); rb_thread_atfork_internal(th, terminate_atfork_i); th->join_list = NULL; - rb_mutex_cleanup_keeping_mutexes(th); rb_fiber_atfork(th); /* We don't want reproduce CVE-2003-0900. */ -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/