From c93adfc170079a54965e939a5a4d57139cd714e1 Mon Sep 17 00:00:00 2001 From: normal Date: Sun, 8 Jul 2018 07:27:24 +0000 Subject: [PATCH] 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 git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@63884 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- mjit.c | 14 ++++++++------ thread.c | 3 +++ thread_pthread.c | 4 +--- thread_win32.c | 2 +- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/mjit.c b/mjit.c index ea615e9481..7a1b3a1383 100644 --- a/mjit.c +++ b/mjit.c @@ -117,7 +117,7 @@ extern void rb_native_cond_signal(rb_nativethread_cond_t *cond); 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) /* 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) 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); diff --git a/thread.c b/thread.c index 360807b073..3943cf0fc6 100644 --- a/thread.c +++ b/thread.c @@ -4276,6 +4276,8 @@ terminate_atfork_i(rb_thread_t *th, const rb_thread_t *current_th) } } +/* mjit.c */ +void mjit_child_after_fork(void); void rb_thread_atfork(void) { @@ -4286,6 +4288,7 @@ rb_thread_atfork(void) /* We don't want reproduce CVE-2003-0900. */ rb_reset_random_seed(); + mjit_child_after_fork(); } static void diff --git a/thread_pthread.c b/thread_pthread.c index 722ce44487..d2ce8fc50b 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1767,14 +1767,12 @@ mjit_worker(void *arg) /* 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 */ diff --git a/thread_win32.c b/thread_win32.c index 3c6d0e7369..2d5eac1ff4 100644 --- a/thread_win32.c +++ b/thread_win32.c @@ -790,7 +790,7 @@ mjit_worker(void *arg) /* 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);