From 74fc05d128c4ad9860740d73b4c7fd43ea18e74a Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Thu, 12 Dec 2024 08:13:42 +0300 Subject: [PATCH] Fix pthread id stored in GC_threads of child process (a cherry-pick of commit bee3ff9c4 from 'master') Issue #682 (bdwgc). `GC_remove_all_threads_but_me()` should also update thread identifiers stored in the table for the current thread. * pthread_support.c [CAN_HANDLE_FORK] (GC_parent_pthread_self): New static variable. * pthread_support.c [CAN_HANDLE_FORK] (GC_remove_all_threads_but_me): Update comment; remove `self` local variable and use `GC_parent_pthread_self` instead; move `me` local variable out of loops; call `store_to_threads_table(hv,NULL)` instead of `store_to_threads_table(hv,me)` inside loop; check `me` is not null after loop; update `me->id` and call `store_to_threads_table(THREAD_TABLE_INDEX(me->id),me)`. * pthread_support.c [CAN_HANDLE_FORK && GC_DARWIN_THREADS] (GC_remove_all_threads_but_me): Move setting `me->stop_info.mach_thread` out of loops. * pthread_support.c [CAN_HANDLE_FORK && USE_TKILL_ON_ANDROID] (GC_remove_all_threads_but_me): Move setting `me->kernel_id` out of loops. * pthread_support.c [CAN_HANDLE_FORK && THREAD_LOCAL_ALLOC && !USE_CUSTOM_SPECIFIC] (GC_remove_all_threads_but_me): Move call `GC_setspecific(GC_thread_key,&me->tlfs)` out of loops. * pthread_support.c [CAN_HANDLE_FORK] (fork_prepare_proc): Set `GC_parent_pthread_self`. * pthread_support.c [CAN_HANDLE_FORK && GC_ASSERTIONS] (fork_parent_proc, fork_child_proc): Clear `GC_parent_pthread_self` at the end of function. --- pthread_support.c | 84 ++++++++++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 30 deletions(-) diff --git a/pthread_support.c b/pthread_support.c index 0e2490fe5..8c74f4545 100644 --- a/pthread_support.c +++ b/pthread_support.c @@ -829,47 +829,28 @@ GC_API void GC_CALL GC_register_altstack(void *stack, GC_word stack_size, GC_threads[hv] = me; } -/* Remove all entries from the GC_threads table, except the */ -/* one for the current thread. We need to do this in the child */ -/* process after a fork(), since only the current thread */ -/* survives in the child. */ +/* Value of pthread_self() of the thread which called fork(). */ +STATIC pthread_t GC_parent_pthread_self; + +/* Remove all entries from the GC_threads table, except the one for */ +/* the current thread. Also update thread identifiers stored in */ +/* the table for the current thread. We need to do this in the */ +/* child process after a fork(), since only the current thread */ +/* survives in the child. */ STATIC void GC_remove_all_threads_but_me(void) { - pthread_t self = pthread_self(); int hv; + GC_thread me = NULL; for (hv = 0; hv < THREAD_TABLE_SZ; ++hv) { GC_thread p, next; - GC_thread me = NULL; for (p = GC_threads[hv]; 0 != p; p = next) { next = p -> next; - if (THREAD_EQUAL(p -> id, self) + if (THREAD_EQUAL(p -> id, GC_parent_pthread_self) && me == NULL) { /* ignore dead threads with the same id */ me = p; p -> next = 0; -# ifdef GC_DARWIN_THREADS - /* Update thread Id after fork (it is OK to call */ - /* GC_destroy_thread_local and GC_free_inner */ - /* before update). */ - me -> stop_info.mach_thread = mach_thread_self(); -# endif -# ifdef USE_TKILL_ON_ANDROID - me -> kernel_id = gettid(); -# endif -# if defined(THREAD_LOCAL_ALLOC) && !defined(USE_CUSTOM_SPECIFIC) - { - int res; - - /* Some TLS implementations might be not fork-friendly, so */ - /* we re-assign thread-local pointer to 'tlfs' for safety */ - /* instead of the assertion check (again, it is OK to call */ - /* GC_destroy_thread_local and GC_free_inner before). */ - res = GC_setspecific(GC_thread_key, &me->tlfs); - if (COVERT_DATAFLOW(res) != 0) - ABORT("GC_setspecific failed (in child)"); - } -# endif } else { # ifdef THREAD_LOCAL_ALLOC if (!(p -> flags & FINISHED)) { @@ -889,8 +870,44 @@ STATIC void GC_remove_all_threads_but_me(void) # endif } } - store_to_threads_table(hv, me); + store_to_threads_table(hv, NULL); + } + +# if defined(CPPCHECK) || defined(LINT2) + if (NULL == me) + ABORT("Current thread is not found after fork"); +# else + GC_ASSERT(me != NULL); +# endif + /* Update pthread's id as it is not guaranteed to be the same */ + /* between this (child) process and the parent one. */ + me -> id = pthread_self(); +# ifdef GC_DARWIN_THREADS + /* Update thread Id after fork (it is OK to call */ + /* GC_destroy_thread_local and GC_free_inner */ + /* before update). */ + me -> stop_info.mach_thread = mach_thread_self(); +# endif +# ifdef USE_TKILL_ON_ANDROID + me -> kernel_id = gettid(); +# endif + + /* Put "me" back to GC_threads. */ + store_to_threads_table(THREAD_TABLE_INDEX(me -> id), me); + +# if defined(THREAD_LOCAL_ALLOC) && !defined(USE_CUSTOM_SPECIFIC) + { + int res; + + /* Some TLS implementations might be not fork-friendly, so */ + /* we re-assign thread-local pointer to 'tlfs' for safety */ + /* instead of the assertion check (again, it is OK to call */ + /* GC_destroy_thread_local and GC_free_inner before). */ + res = GC_setspecific(GC_thread_key, &me->tlfs); + if (COVERT_DATAFLOW(res) != 0) + ABORT("GC_setspecific failed (in child)"); } +# endif } #endif /* CAN_HANDLE_FORK */ @@ -1190,6 +1207,7 @@ static void fork_prepare_proc(void) /* the (one remaining thread in) the child. */ LOCK(); DISABLE_CANCEL(fork_cancel_state); + GC_parent_pthread_self = pthread_self(); /* Following waits may include cancellation points. */ # if defined(PARALLEL_MARK) if (GC_parallel) @@ -1230,6 +1248,9 @@ static void fork_parent_proc(void) } # endif RESTORE_CANCEL(fork_cancel_state); +# ifdef GC_ASSERTIONS + BZERO(&GC_parent_pthread_self, sizeof(pthread_t)); +# endif UNLOCK(); } @@ -1272,6 +1293,9 @@ static void fork_child_proc(void) /* Clean up the thread table, so that just our thread is left. */ GC_remove_all_threads_but_me(); RESTORE_CANCEL(fork_cancel_state); +# ifdef GC_ASSERTIONS + BZERO(&GC_parent_pthread_self, sizeof(pthread_t)); +# endif UNLOCK(); /* Even though after a fork the child only inherits the single */ /* thread that called the fork(), if another thread in the parent */