diff options
author | Santosh Mahto <santoshmahto@collabora.co.uk> | 2017-10-16 08:45:07 (GMT) |
---|---|---|
committer | Santosh Mahto <santoshmahto@collabora.co.uk> | 2017-10-17 14:07:21 (GMT) |
commit | 7aaf1b4a91dc15e9d89085c83d7c992f03519d02 (patch) | |
tree | 1c1c6b8fa2c71ce5ef854803280be0ebd7cec1c5 | |
parent | 73f6346e4c3ac79f5bf306e265ea515a41123e83 (diff) | |
download | chromium-7aaf1b4a91dc15e9d89085c83d7c992f03519d02.tar.gz chromium-7aaf1b4a91dc15e9d89085c83d7c992f03519d02.tar.xz |
Startup hang issue fix.
This issue solves race condition happening while forking when
services are launched. There were two issue observed:
1. One of fork fails when UI and Browser service processes are launched.
2. tcmalloc gets hanged while forking
(https://github.com/gperftools/gperftools/issues/499)
-rw-r--r-- | base/process/launch_posix.cc | 5 | ||||
-rw-r--r-- | third_party/tcmalloc/chromium/src/central_freelist.h | 10 | ||||
-rw-r--r-- | third_party/tcmalloc/chromium/src/static_vars.cc | 31 |
3 files changed, 46 insertions, 0 deletions
diff --git a/base/process/launch_posix.cc b/base/process/launch_posix.cc index 7550a36..898191e 100644 --- a/base/process/launch_posix.cc +++ b/base/process/launch_posix.cc @@ -341,6 +341,10 @@ Process LaunchProcess(const std::vector<std::string>& argv, } pid_t pid; + // Note: Forking here fails due to multithreaded environment. Trying to avoid + // this using lock. + static base::Lock fork_lock; + fork_lock.Acquire(); #if defined(OS_LINUX) || defined(OS_AIX) if (options.clone_flags) { // Signal handling in this function assumes the creation of a new @@ -365,6 +369,7 @@ Process LaunchProcess(const std::vector<std::string>& argv, pid = fork(); } + fork_lock.Release(); // Always restore the original signal mask in the parent. if (pid != 0) { SetSignalMask(orig_sigmask); diff --git a/third_party/tcmalloc/chromium/src/central_freelist.h b/third_party/tcmalloc/chromium/src/central_freelist.h index 4fd5799..e547f15 100644 --- a/third_party/tcmalloc/chromium/src/central_freelist.h +++ b/third_party/tcmalloc/chromium/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/third_party/tcmalloc/chromium/src/static_vars.cc b/third_party/tcmalloc/chromium/src/static_vars.cc index 6fc852a..35e0fa9 100644 --- a/third_party/tcmalloc/chromium/src/static_vars.cc +++ b/third_party/tcmalloc/chromium/src/static_vars.cc @@ -33,6 +33,9 @@ #include "static_vars.h" #include <stddef.h> // for NULL #include <new> // for operator new +#ifdef HAVE_PTHREAD +#include <pthread.h> // for pthread_atfork +#endif #include "internal_logging.h" // for CHECK_CONDITION #include "common.h" #include "sampler.h" // for Sampler @@ -49,6 +52,27 @@ PageHeapAllocator<StackTraceTable::Bucket> Static::bucket_allocator_; StackTrace* Static::growth_stacks_ = NULL; PageHeap* Static::pageheap_ = NULL; +#ifdef HAVE_PTHREAD +// These following two functions are registered via pthread_atfork to make +// sure the central_cache locks remain in a consistent state in the forked +// version of the thread. + +void CentralCacheLockAll() +{ + Static::pageheap_lock()->Lock(); + for (int i = 0; i < kNumClasses; ++i) + Static::central_cache()[i].Lock(); +} + +void CentralCacheUnlockAll() +{ + for (int i = 0; i < kNumClasses; ++i) + Static::central_cache()[i].Unlock(); + Static::pageheap_lock()->Unlock(); +} + +#endif + void Static::InitStaticVars() { sizemap_.Init(); span_allocator_.Init(); @@ -61,6 +85,13 @@ void Static::InitStaticVars() { for (int i = 0; i < kNumClasses; ++i) { central_cache_[i].Init(i); } + +#if defined(HAVE_PTHREAD) && !defined(__APPLE__) + pthread_atfork(CentralCacheLockAll, // parent calls before fork + CentralCacheUnlockAll, // parent calls after fork + CentralCacheUnlockAll); // child calls after fork +#endif + // 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, |