[前][次][番号順一覧][スレッド一覧]

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/

[前][次][番号順一覧][スレッド一覧]