From 05c33f53081cf737852f86bf3e19891e99043103 Mon Sep 17 00:00:00 2001 From: Adhemerval Zanella Date: Sun, 2 Feb 2014 19:21:53 -0200 Subject: [PATCH] issue-496: Fix possible deadlock in thread+fork This patch adds a pthread_atfork handler to set the internal locks in consistent state. --- configure | 11 +++++++++++ configure.ac | 1 + src/central_freelist.h | 10 ++++++++++ src/config.h.in | 3 +++ src/static_vars.cc | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+) diff --git a/configure b/configure index a5c1763..1b7c5cf 100755 --- a/configure +++ b/configure @@ -15114,6 +15114,17 @@ _ACEOF fi done # for turning off services when run as root +for ac_func in fork +do : + ac_fn_c_check_func "$LINENO" "fork" "ac_cv_func_fork" +if test "x$ac_cv_func_fork" = xyes; then : + cat >>confdefs.h <<_ACEOF +#define HAVE_FORK 1 +_ACEOF + +fi +done + # for the pthread_atfork setup for ac_header in features.h do : ac_fn_c_check_header_mongrel "$LINENO" "features.h" "ac_cv_header_features_h" "$ac_includes_default" diff --git a/configure.ac b/configure.ac index efe3217..008ea97 100644 --- a/configure.ac +++ b/configure.ac @@ -134,6 +134,7 @@ AC_CHECK_TYPES([struct mallinfo],,, [#include ]) AC_CHECK_TYPES([Elf32_Versym],,, [#include ]) # for vdso_support.h AC_CHECK_FUNCS(sbrk) # for tcmalloc to get memory AC_CHECK_FUNCS(geteuid) # for turning off services when run as root +AC_CHECK_FUNCS(fork) # for the pthread_atfork setup AC_CHECK_HEADERS(features.h) # for vdso_support.h AC_CHECK_HEADERS(malloc.h) # some systems define stuff there, others not AC_CHECK_HEADERS(sys/malloc.h) # where some versions of OS X put malloc.h diff --git a/src/central_freelist.h b/src/central_freelist.h index 4fd5799..e547f15 100644 --- a/src/central_freelist.h +++ b/src/central_freelist.h @@ -79,6 +79,16 @@ class CentralFreeList { // page full of 5-byte objects would have 2 bytes memory overhead). size_t OverheadBytes(); + // Lock/Unlock the internal SpinLock. Used on the pthread_atfork call + // to set the lock in a consistent state before the fork. + void Lock() { + lock_.Lock(); + } + + void Unlock() { + lock_.Unlock(); + } + private: // TransferCache is used to cache transfers of // sizemap.num_objects_to_move(size_class) back and forth between diff --git a/src/config.h.in b/src/config.h.in index b13d0c8..46bc9f9 100644 --- a/src/config.h.in +++ b/src/config.h.in @@ -56,6 +56,9 @@ /* Define to 1 if you have the header file. */ #undef HAVE_FEATURES_H +/* Define to 1 if you have the `fork' function. */ +#undef HAVE_FORK + /* Define to 1 if you have the `geteuid' function. */ #undef HAVE_GETEUID diff --git a/src/static_vars.cc b/src/static_vars.cc index 6fc852a..8ccad04 100644 --- a/src/static_vars.cc +++ b/src/static_vars.cc @@ -39,6 +39,36 @@ namespace tcmalloc { +#if defined(HAVE_FORK) && defined(HAVE_PTHREAD) +// These following two functions are registered via pthread_atfork to make +// sure the central_cache locks remain in a consisten state in the forked +// version of the thread. + +static void CentralCacheLockAll() +{ + Static::pageheap_lock()->Lock(); + for (int i = 0; i < kNumClasses; ++i) + Static::central_cache()[i].Lock(); +} + +static void CentralCacheUnlockAll() +{ + for (int i = 0; i < kNumClasses; ++i) + Static::central_cache()[i].Unlock(); + Static::pageheap_lock()->Unlock(); +} +#endif + +static inline void SetupAtForkLocksHandler() +{ +#if defined(HAVE_FORK) && defined(HAVE_PTHREAD) + pthread_atfork(CentralCacheLockAll, // parent calls before fork + CentralCacheUnlockAll, // parent calls after fork + CentralCacheUnlockAll); // child calls after fork +#endif +} + + SpinLock Static::pageheap_lock_(SpinLock::LINKER_INITIALIZED); SizeMap Static::sizemap_; CentralFreeListPadded Static::central_cache_[kNumClasses]; @@ -49,6 +79,7 @@ PageHeapAllocator Static::bucket_allocator_; StackTrace* Static::growth_stacks_ = NULL; PageHeap* Static::pageheap_ = NULL; + void Static::InitStaticVars() { sizemap_.Init(); span_allocator_.Init(); @@ -61,6 +92,8 @@ void Static::InitStaticVars() { for (int i = 0; i < kNumClasses; ++i) { central_cache_[i].Init(i); } + SetupAtForkLocksHandler(); + // It's important to have PageHeap allocated, not in static storage, // so that HeapLeakChecker does not consider all the byte patterns stored // in is caches as pointers that are sources of heap object liveness,