From 2cd0f5e56718a053341d1b5e7df7a3cab9a5ceaa Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Fri, 13 Dec 2024 00:26:38 +0300 Subject: [PATCH] Fix pthread id stored in key thread_specific_data of child process (a cherry-pick of commit fa63cef86 from 'master') Issue #682 (bdwgc). Update thread-specific data for the survived thread of the child process. This is needed because `pthread_self()` value is not guaranteed to be the same between the child and parent processes. Matters only if `USE_CUSTOM_SPECIFIC` macro is defined. * include/private/gc_priv.h [CAN_HANDLE_FORK && GC_PTHREADS]: Include `pthread.h`. * include/private/gc_priv.h [CAN_HANDLE_FORK && GC_PTHREADS] (GC_arrays._parent_pthread_self): New field; add comment. * include/private/gc_priv.h [CAN_HANDLE_FORK && GC_PTHREADS] (GC_parent_pthread_self): Define as a macro. * include/private/specific.h [CAN_HANDLE_FORK] (GC_update_specific_after_fork): Declare function. * pthread_support.c [CAN_HANDLE_FORK] (GC_parent_pthread_self): Remove static variable. * pthread_support.c [CAN_HANDLE_FORK && THREAD_LOCAL_ALLOC && USE_CUSTOM_SPECIFIC] (GC_remove_all_threads_but_me): Call `GC_update_specific_after_fork()` (after `store_to_threads_table()` call). * specific.c [USE_CUSTOM_SPECIFIC && CAN_HANDLE_FORK] (GC_update_specific_after_fork): Implement. --- include/private/gc_priv.h | 11 +++++++++++ include/private/specific.h | 7 +++++++ pthread_support.c | 31 ++++++++++++++++--------------- specific.c | 27 +++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index 92f405a2a..729526c73 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -842,6 +842,10 @@ EXTERN_C_END #include +#if defined(CAN_HANDLE_FORK) && defined(GC_PTHREADS) +# include /* for pthread_t */ +#endif + #if __STDC_VERSION__ >= 201112L # include /* for static_assert */ #endif @@ -1562,6 +1566,13 @@ struct _GC_arrays { # endif size_t _ed_size; /* Current size of above arrays. */ size_t _avail_descr; /* Next available slot. */ + +# if defined(CAN_HANDLE_FORK) && defined(GC_PTHREADS) + /* Value of pthread_self() of the thread which called fork(). */ +# define GC_parent_pthread_self GC_arrays._parent_pthread_self + pthread_t _parent_pthread_self; +# endif + typed_ext_descr_t *_ext_descriptors; /* Points to array of extended */ /* descriptors. */ GC_mark_proc _mark_procs[MAX_MARK_PROCS]; diff --git a/include/private/specific.h b/include/private/specific.h index 3ed75647c..029fc7f3d 100644 --- a/include/private/specific.h +++ b/include/private/specific.h @@ -109,6 +109,13 @@ GC_INNER int GC_setspecific(tsd * key, void * value); GC_remove_specific_after_fork(key, pthread_self()) GC_INNER void GC_remove_specific_after_fork(tsd * key, pthread_t t); +#ifdef CAN_HANDLE_FORK + /* Update thread-specific data for the survived thread of the child */ + /* process. Should be called once after removing thread-specific data */ + /* for other threads. */ + GC_INNER void GC_update_specific_after_fork(tsd *key); +#endif + /* An internal version of getspecific that assumes a cache miss. */ GC_INNER void * GC_slow_getspecific(tsd * key, word qtid, tse * volatile * cache_entry); diff --git a/pthread_support.c b/pthread_support.c index cc99ebeeb..25a37e62a 100644 --- a/pthread_support.c +++ b/pthread_support.c @@ -829,9 +829,6 @@ GC_API void GC_CALL GC_register_altstack(void *stack, GC_word stack_size, GC_threads[hv] = me; } -/* 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 */ @@ -895,18 +892,22 @@ STATIC void GC_remove_all_threads_but_me(void) /* 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)"); - } +# ifdef THREAD_LOCAL_ALLOC +# ifdef USE_CUSTOM_SPECIFIC + GC_update_specific_after_fork(GC_thread_key); +# else + { + 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 } #endif /* CAN_HANDLE_FORK */ diff --git a/specific.c b/specific.c index 09387ac3a..1293d40b4 100644 --- a/specific.c +++ b/specific.c @@ -133,6 +133,33 @@ GC_INNER void GC_remove_specific_after_fork(tsd * key, pthread_t t) ABORT("pthread_mutex_unlock failed (remove_specific after fork)"); } +#ifdef CAN_HANDLE_FORK + GC_INNER void + GC_update_specific_after_fork(tsd *key) + { + unsigned hash_val = HASH(GC_parent_pthread_self); + tse *entry; + + GC_ASSERT(I_HOLD_LOCK()); +# ifdef LINT2 + pthread_mutex_lock(&key->lock); +# endif + entry = key->hash[hash_val].p; + if (EXPECT(entry != NULL, TRUE)) { + GC_ASSERT(THREAD_EQUAL(entry->thread, GC_parent_pthread_self)); + GC_ASSERT(NULL == entry->next); + /* Remove the entry from the table. */ + key->hash[hash_val].p = NULL; + entry->thread = pthread_self(); + /* Then put the entry back to the table (based on new hash value). */ + key->hash[HASH(entry->thread)].p = entry; + } +# ifdef LINT2 + (void)pthread_mutex_unlock(&key->lock); +# endif + } +#endif + /* Note that even the slow path doesn't lock. */ GC_INNER void * GC_slow_getspecific(tsd * key, word qtid, tse * volatile * cache_ptr)