From 6383cee08869517ec3dee257d0adb582c4fc7805 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Mon, 28 Aug 2017 09:27:07 +0300 Subject: [PATCH] Fix GC_remove_specific invocation from remove_all_threads_but_me (fix commits fcfae7f, 38e65ea) * include/private/specific.h (GC_remove_specific): Define as macro (calls GC_remove_specific_after_fork). * include/private/specific.h (GC_remove_specific_after_fork): New GC_INNER function declaration. * include/private/thread_local_alloc.h [USE_PTHREAD_SPECIFIC || USE_COMPILER_TLS || USE_WIN32_COMPILER_TLS || USE_WIN32_SPECIFIC] (GC_remove_specific_after_fork): New macro (defined to no-op). * pthread_support.c [CAN_HANDLE_FORK && THREAD_LOCAL_ALLOC] (GC_remove_all_threads_but_me): Call GC_remove_specific_after_fork instead of GC_remove_specific (i.e. delete thread-specific control data for the given thread instead of the current one). * win32_threads.c [CAN_HANDLE_FORK && THREAD_LOCAL_ALLOC] Like * specific.c [USE_CUSTOM_SPECIFIC] (GC_setspecific): Add assertion that the allocation lock is held; remove the corresponding comment. * specific.c [USE_CUSTOM_SPECIFIC] (GC_remove_specific): Replace to GC_remove_specific_after_fork (add t argument); replace self local variable with t argument; update comment; * specific.c [USE_CUSTOM_SPECIFIC && CAN_HANDLE_FORK] (GC_remove_specific_after_fork): Add assertion that the allocation lock is held; add comment. --- include/private/specific.h | 4 +++- include/private/thread_local_alloc.h | 4 ++++ pthread_support.c | 2 +- specific.c | 20 +++++++++++++------- win32_threads.c | 2 +- 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/include/private/specific.h b/include/private/specific.h index 7df6ce56..0d100a4c 100644 --- a/include/private/specific.h +++ b/include/private/specific.h @@ -77,7 +77,9 @@ typedef tsd * GC_key_t; #define GC_key_create(key, d) GC_key_create_inner(key) GC_INNER int GC_key_create_inner(tsd ** key_ptr); GC_INNER int GC_setspecific(tsd * key, void * value); -GC_INNER void GC_remove_specific(tsd * key); +#define GC_remove_specific(key) \ + GC_remove_specific_after_fork(key, pthread_self()) +GC_INNER void GC_remove_specific_after_fork(tsd * key, pthread_t t); /* An internal version of getspecific that assumes a cache miss. */ GC_INNER void * GC_slow_getspecific(tsd * key, word qtid, diff --git a/include/private/thread_local_alloc.h b/include/private/thread_local_alloc.h index 1b71cbf5..cc0ec9f2 100644 --- a/include/private/thread_local_alloc.h +++ b/include/private/thread_local_alloc.h @@ -117,12 +117,15 @@ typedef struct thread_local_freelists { # define GC_remove_specific(key) pthread_setspecific(key, NULL) /* Explicitly delete the value to stop the TLS */ /* destructor from being called repeatedly. */ +# define GC_remove_specific_after_fork(key, t) (void)0 + /* Should not need any action. */ typedef pthread_key_t GC_key_t; #elif defined(USE_COMPILER_TLS) || defined(USE_WIN32_COMPILER_TLS) # define GC_getspecific(x) (x) # define GC_setspecific(key, v) ((key) = (v), 0) # define GC_key_create(key, d) 0 # define GC_remove_specific(key) /* No need for cleanup on exit. */ +# define GC_remove_specific_after_fork(key, t) (void)0 typedef void * GC_key_t; #elif defined(USE_WIN32_SPECIFIC) # ifndef WIN32_LEAN_AND_MEAN @@ -141,6 +144,7 @@ typedef struct thread_local_freelists { ((d) != 0 || (*(key) = TlsAlloc()) == TLS_OUT_OF_INDEXES ? -1 : 0) # define GC_remove_specific(key) /* No need for cleanup on exit. */ /* Need TlsFree on process exit/detach? */ +# define GC_remove_specific_after_fork(key, t) (void)0 typedef DWORD GC_key_t; #elif defined(USE_CUSTOM_SPECIFIC) # include "private/specific.h" diff --git a/pthread_support.c b/pthread_support.c index 7b02d95c..b8bf1876 100644 --- a/pthread_support.c +++ b/pthread_support.c @@ -783,7 +783,7 @@ STATIC void GC_remove_all_threads_but_me(void) # ifdef THREAD_LOCAL_ALLOC if (!(p -> flags & FINISHED)) { GC_destroy_thread_local(&(p->tlfs)); - GC_remove_specific(GC_thread_key); + GC_remove_specific_after_fork(GC_thread_key, p -> id); } # endif if (p != &first_thread) GC_INTERNAL_FREE(p); diff --git a/specific.c b/specific.c index f787fa59..eca2d937 100644 --- a/specific.c +++ b/specific.c @@ -46,13 +46,13 @@ GC_INNER int GC_key_create_inner(tsd ** key_ptr) return 0; } -/* Called with the lock held. */ GC_INNER int GC_setspecific(tsd * key, void * value) { pthread_t self = pthread_self(); int hash_val = HASH(self); volatile tse * entry; + GC_ASSERT(I_HOLD_LOCK()); GC_ASSERT(self != INVALID_THREADID); GC_dont_gc++; /* disable GC */ entry = (volatile tse *)MALLOC_CLEAR(sizeof(tse)); @@ -72,18 +72,24 @@ GC_INNER int GC_setspecific(tsd * key, void * value) return 0; } -/* Remove thread-specific data for this thread. Should be called on */ -/* thread exit. */ -GC_INNER void GC_remove_specific(tsd * key) +/* Remove thread-specific data for a given thread. This function is */ +/* called at fork from the child process for all threads except for the */ +/* survived one. GC_remove_specific() should be called on thread exit. */ +GC_INNER void GC_remove_specific_after_fork(tsd * key, pthread_t t) { - pthread_t self = pthread_self(); - unsigned hash_val = HASH(self); + unsigned hash_val = HASH(t); tse *entry; tse **link = &key->hash[hash_val].p; +# ifdef CAN_HANDLE_FORK + /* Both GC_setspecific and GC_remove_specific should be called */ + /* with the allocation lock held to ensure the consistency of */ + /* the hash table in the forked child. */ + GC_ASSERT(I_HOLD_LOCK()); +# endif pthread_mutex_lock(&(key -> lock)); entry = *link; - while (entry != NULL && entry -> thread != self) { + while (entry != NULL && entry -> thread != t) { link = &(entry -> next); entry = *link; } diff --git a/win32_threads.c b/win32_threads.c index 3cbdc22c..36cf0016 100644 --- a/win32_threads.c +++ b/win32_threads.c @@ -1039,7 +1039,7 @@ GC_API void * GC_CALL GC_call_with_gc_active(GC_fn_type fn, # ifdef THREAD_LOCAL_ALLOC if ((p -> flags & FINISHED) == 0) { GC_destroy_thread_local(&p->tlfs); - GC_remove_specific(GC_thread_key); + GC_remove_specific_after_fork(GC_thread_key, p -> pthread_id); } # endif if (&first_thread != p) -- 2.40.0