From 6e36841dbd5f52b572b690b8a4c3c534fec43ba8 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Wed, 11 Jun 2025 16:21:11 -0400 Subject: [PATCH] Free rb_native_thread memory at fork We never freed any resources of rb_native_thread at fork because it would cause it to hang. This is because it called rb_native_cond_destroy for condition variables. We can't call rb_native_cond_destroy here because according to the specs of pthread_cond_destroy: Attempting to destroy a condition variable upon which other threads are currently blocked results in undefined behavior. Specifically, glibc's pthread_cond_destroy waits on all the other listeners. Since after forking all the threads are dead, the condition variable's listeners will never wake up, so it will hang forever. This commit changes it to only free the memory and none of the condition variables. --- thread.c | 6 +----- thread_none.c | 6 ++++++ thread_pthread.c | 25 ++++++++++++++++++++++--- thread_win32.c | 6 ++++++ 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/thread.c b/thread.c index a637c8ec7c..d79603d64b 100644 --- a/thread.c +++ b/thread.c @@ -519,12 +519,8 @@ thread_cleanup_func(void *th_ptr, int atfork) th->locking_mutex = Qfalse; thread_cleanup_func_before_exec(th_ptr); - /* - * Unfortunately, we can't release native threading resource at fork - * because libc may have unstable locking state therefore touching - * a threading resource may cause a deadlock. - */ if (atfork) { + native_thread_destroy_atfork(th->nt); th->nt = NULL; return; } diff --git a/thread_none.c b/thread_none.c index d535d9af4c..38686e17c1 100644 --- a/thread_none.c +++ b/thread_none.c @@ -137,6 +137,12 @@ ruby_mn_threads_params(void) { } +static void +native_thread_destroy_atfork(struct rb_native_thread *nt) +{ + /* no-op */ +} + static int native_thread_init_stack(rb_thread_t *th, void *local_in_parent_frame) { diff --git a/thread_pthread.c b/thread_pthread.c index f9352bbb56..377e1d9f64 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1816,6 +1816,27 @@ native_thread_assign(struct rb_native_thread *nt, rb_thread_t *th) th->nt = nt; } +static void +native_thread_destroy_atfork(struct rb_native_thread *nt) +{ + if (nt) { + /* We can't call rb_native_cond_destroy here because according to the + * specs of pthread_cond_destroy: + * + * Attempting to destroy a condition variable upon which other threads + * are currently blocked results in undefined behavior. + * + * Specifically, glibc's pthread_cond_destroy waits on all the other + * listeners. Since after forking all the threads are dead, the condition + * variable's listeners will never wake up, so it will hang forever. + */ + + RB_ALTSTACK_FREE(nt->altstack); + ruby_xfree(nt->nt_context); + ruby_xfree(nt); + } +} + static void native_thread_destroy(struct rb_native_thread *nt) { @@ -1826,9 +1847,7 @@ native_thread_destroy(struct rb_native_thread *nt) rb_native_cond_destroy(&nt->cond.intr); } - RB_ALTSTACK_FREE(nt->altstack); - ruby_xfree(nt->nt_context); - ruby_xfree(nt); + native_thread_destroy_atfork(nt); } } diff --git a/thread_win32.c b/thread_win32.c index ed8a99dd88..576f617e8d 100644 --- a/thread_win32.c +++ b/thread_win32.c @@ -617,6 +617,12 @@ native_thread_init_stack(rb_thread_t *th, void *local_in_parent_frame) th->ec->machine.stack_maxsize = size - space; } +static void +native_thread_destroy_atfork(struct rb_native_thread *nt) +{ + /* no-op */ +} + #ifndef InterlockedExchangePointer #define InterlockedExchangePointer(t, v) \ (void *)InterlockedExchange((long *)(t), (long)(v))