normal	2018-07-08 16:27:24 +0900 (Sun, 08 Jul 2018)

  New Revision: 63884

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=63884

  Log:
    mjit: get rid of memory leak in pause+resume loop
    
    pthread_atfork is not idempotent and repeatedly calling it
    causes it to register the same hook repeatedly; leading to
    unbound memory growth.
    
    Ruby already has a (confusing-named) internal API for to call
    in the forked child process: rb_thread_atfork
    Call the MJIT child_after_fork hook inside that to prevent
    unbound growth with the following loop:
    
        loop do
          RubyVM::MJIT.pause
          RubyVM::MJIT.resume
        end

  Modified files:
    trunk/mjit.c
    trunk/thread.c
    trunk/thread_pthread.c
    trunk/thread_win32.c
Index: thread_win32.c
===================================================================
--- thread_win32.c	(revision 63883)
+++ thread_win32.c	(revision 63884)
@@ -790,7 +790,7 @@ mjit_worker(void *arg) https://github.com/ruby/ruby/blob/trunk/thread_win32.c#L790
 
 /* Launch MJIT thread. Returns FALSE if it fails to create thread. */
 int
-rb_thread_create_mjit_thread(void (*child_hook)(void), void (*worker_func)(void))
+rb_thread_create_mjit_thread(void (*worker_func)(void))
 {
     size_t stack_size = 4 * 1024; /* 4KB is the minimum commit size */
     HANDLE thread_id = w32_create_thread(stack_size, mjit_worker, worker_func);
Index: thread_pthread.c
===================================================================
--- thread_pthread.c	(revision 63883)
+++ thread_pthread.c	(revision 63884)
@@ -1767,14 +1767,12 @@ mjit_worker(void *arg) https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L1767
 
 /* Launch MJIT thread. Returns FALSE if it fails to create thread. */
 int
-rb_thread_create_mjit_thread(void (*child_hook)(void), void (*worker_func)(void))
+rb_thread_create_mjit_thread(void (*worker_func)(void))
 {
     pthread_attr_t attr;
     pthread_t worker_pid;
     int ret = FALSE;
 
-    pthread_atfork(NULL, NULL, child_hook);
-
     if (pthread_attr_init(&attr) != 0) return ret;
 
     /* jit_worker thread is not to be joined */
Index: thread.c
===================================================================
--- thread.c	(revision 63883)
+++ thread.c	(revision 63884)
@@ -4276,6 +4276,8 @@ terminate_atfork_i(rb_thread_t *th, cons https://github.com/ruby/ruby/blob/trunk/thread.c#L4276
     }
 }
 
+/* mjit.c */
+void mjit_child_after_fork(void);
 void
 rb_thread_atfork(void)
 {
@@ -4286,6 +4288,7 @@ rb_thread_atfork(void) https://github.com/ruby/ruby/blob/trunk/thread.c#L4288
 
     /* We don't want reproduce CVE-2003-0900. */
     rb_reset_random_seed();
+    mjit_child_after_fork();
 }
 
 static void
Index: mjit.c
===================================================================
--- mjit.c	(revision 63883)
+++ mjit.c	(revision 63884)
@@ -117,7 +117,7 @@ extern void rb_native_cond_signal(rb_nat https://github.com/ruby/ruby/blob/trunk/mjit.c#L117
 extern void rb_native_cond_broadcast(rb_nativethread_cond_t *cond);
 extern void rb_native_cond_wait(rb_nativethread_cond_t *cond, rb_nativethread_lock_t *mutex);
 
-extern int rb_thread_create_mjit_thread(void (*child_hook)(void), void (*worker_func)(void));
+extern int rb_thread_create_mjit_thread(void (*worker_func)(void));
 
 /* process.c */
 rb_pid_t ruby_waitpid_locked(rb_vm_t *, rb_pid_t, int *status, int options,
@@ -1319,11 +1319,13 @@ init_header_filename(void) https://github.com/ruby/ruby/blob/trunk/mjit.c#L1319
 
 /* This is called after each fork in the child in to switch off MJIT
    engine in the child as it does not inherit MJIT threads.  */
-static void
-child_after_fork(void)
+void
+mjit_child_after_fork(void)
 {
-    verbose(3, "Switching off MJIT in a forked child");
-    mjit_enabled = FALSE;
+    if (mjit_enabled) {
+        verbose(3, "Switching off MJIT in a forked child");
+        mjit_enabled = FALSE;
+    }
     /* TODO: Should we initiate MJIT in the forked Ruby.  */
 }
 
@@ -1433,7 +1435,7 @@ start_worker(void) https://github.com/ruby/ruby/blob/trunk/mjit.c#L1435
     stop_worker_p = FALSE;
     worker_stopped = FALSE;
 
-    if (!rb_thread_create_mjit_thread(child_after_fork, worker)) {
+    if (!rb_thread_create_mjit_thread(worker)) {
         mjit_enabled = FALSE;
         rb_native_mutex_destroy(&mjit_engine_mutex);
         rb_native_cond_destroy(&mjit_pch_wakeup);

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