Fix malloc deadlock caused by contention profiler

src/brpc/builtin/hotspots_service.cpp

@@ -33,8 +33,8 @@
 #include "brpc/details/tcmalloc_extension.h"
 
 extern "C" {
-int __attribute__((weak)) ProfilerStart(const char* fname);
-void __attribute__((weak)) ProfilerStop();
+int BAIDU_WEAK ProfilerStart(const char* fname);
+void BAIDU_WEAK ProfilerStop();
 }
 
 namespace bthread {

src/brpc/builtin/pprof_service.cpp

@@ -41,8 +41,8 @@ extern "C" {
 #if defined(OS_LINUX)
 extern char *program_invocation_name;
 #endif
-int __attribute__((weak)) ProfilerStart(const char* fname);
-void __attribute__((weak)) ProfilerStop();
+int BAIDU_WEAK ProfilerStart(const char* fname);
+void BAIDU_WEAK ProfilerStop();
 }
 
 namespace bthread {

src/brpc/policy/rtmp_protocol.cpp

@@ -39,7 +39,7 @@
 // we mark the symbol as weak. If the runtime does not have the function,
 // handshaking will fallback to the simple one.
 extern "C" {
-const EVP_MD* __attribute__((weak)) EVP_sha256(void);
+const EVP_MD* BAIDU_WEAK EVP_sha256(void);
 }
 
 

src/brpc/socket.cpp

@@ -57,8 +57,7 @@
 #endif
 
 namespace bthread {
-size_t __attribute__((weak))
-get_sizes(const bthread_id_list_t* list, size_t* cnt, size_t n);
+size_t BAIDU_WEAK get_sizes(const bthread_id_list_t* list, size_t* cnt, size_t n);
 }
 
 

src/bthread/mutex.cpp

@@ -20,7 +20,6 @@
 // Date: Sun Aug  3 12:46:15 CST 2014
 
 #include <pthread.h>
-#include <execinfo.h>
 #include <dlfcn.h>                               // dlsym
 #include <fcntl.h>                               // O_RDONLY
 #include "butil/atomicops.h"
@@ -34,26 +33,25 @@
 #include "butil/files/file_path.h"
 #include "butil/file_util.h"
 #include "butil/unique_ptr.h"
+#include "butil/memory/scope_guard.h"
 #include "butil/third_party/murmurhash3/murmurhash3.h"
+#include "butil/third_party/symbolize/symbolize.h"
 #include "butil/logging.h"
 #include "butil/object_pool.h"
+#include "butil/debug/stack_trace.h"
 #include "bthread/butex.h"                       // butex_*
 #include "bthread/mutex.h"                       // bthread_mutex_t
 #include "bthread/sys_futex.h"
 #include "bthread/log.h"
 #include "butil/debug/stack_trace.h"
 
 extern "C" {
-extern void* __attribute__((weak)) _dl_sym(void* handle, const char* symbol, void* caller);
+extern void* BAIDU_WEAK _dl_sym(void* handle, const char* symbol, void* caller);
 }
-extern int __attribute__((weak)) GetStackTrace(void** result, int max_depth, int skip_count);
 
 namespace bthread {
 // Warm up backtrace before main().
-void* dummy_buf[4];
-const int ALLOW_UNUSED dummy_bt = GetStackTrace
-    ? GetStackTrace(dummy_buf, arraysize(dummy_buf), 0)
-    : backtrace(dummy_buf, arraysize(dummy_buf));
+const butil::debug::StackTrace ALLOW_UNUSED dummy_bt;
 
 // For controlling contentions collected per second.
 static bvar::CollectorSpeedLimit g_cp_sl = BVAR_COLLECTOR_SPEED_LIMIT_INITIALIZER;
@@ -468,6 +466,10 @@ inline uint64_t hash_mutex_ptr(const Mutex* m) {
 // code are never sampled, otherwise deadlock may occur.
 static __thread bool tls_inside_lock = false;
 
+// Warn up some singleton objects used in contention profiler
+// to avoid deadlock in malloc call stack.
+static __thread bool tls_warn_up = false;
+
 // ++tls_pthread_lock_count when pthread locking,
 // --tls_pthread_lock_count when pthread unlocking.
 // Only when it is equal to 0, it is safe for the bthread to be scheduled.
@@ -559,18 +561,37 @@ inline bool remove_pthread_contention_site(const Mutex* mutex,
 // Submit the contention along with the callsite('s stacktrace)
 void submit_contention(const bthread_contention_site_t& csite, int64_t now_ns) {
     tls_inside_lock = true;
+    BRPC_SCOPE_EXIT {
+        tls_inside_lock = false;
+    };
+
+    butil::debug::StackTrace stack(true); // May lock.
+    if (0 == stack.FrameCount()) {
+        return;
+    }
+    // There are two situations where we need to check whether in the
+    // malloc call stack:
+    // 1. Warn up some singleton objects used in `submit_contention'
+    // to avoid deadlock in malloc call stack.
+    // 2. LocalPool is empty, GlobalPool may allocate memory by malloc.
+    if (!tls_warn_up || butil::local_pool_free_empty<SampledContention>()) {
+        // In malloc call stack, can not submit contention.
+        if (stack.FindSymbol((void*)malloc)) {
+            return;
+        }
+    }
+
     auto sc = butil::get_object<SampledContention>();
     // Normalize duration_us and count so that they're addable in later
     // processings. Notice that sampling_range is adjusted periodically by
     // collecting thread.
     sc->duration_ns = csite.duration_ns * bvar::COLLECTOR_SAMPLING_BASE
         / csite.sampling_range;
     sc->count = bvar::COLLECTOR_SAMPLING_BASE / (double)csite.sampling_range;
-    sc->nframes = GetStackTrace
-        ? GetStackTrace(sc->stack, arraysize(sc->stack), 0)
-        : backtrace(sc->stack, arraysize(sc->stack)); // may lock
+    sc->nframes = stack.CopyAddressTo(sc->stack, arraysize(sc->stack));
     sc->submit(now_ns / 1000);  // may lock
-    tls_inside_lock = false;
+    // Once submit a contention, complete warn up.
+    tls_warn_up = true;
 }
 
 namespace internal {

src/butil/debug/stack_trace.cc

@@ -21,16 +21,19 @@ StackTrace::StackTrace(const void* const* trace, size_t count) {
   count_ = count;
 }
 
-StackTrace::~StackTrace() {
-}
-
 const void *const *StackTrace::Addresses(size_t* count) const {
   *count = count_;
   if (count_)
     return trace_;
   return NULL;
 }
 
+size_t StackTrace::CopyAddressTo(void** buffer, size_t max_nframes) const {
+    size_t nframes = std::min(count_, max_nframes);
+    memcpy(buffer, trace_, nframes * sizeof(void*));
+    return nframes;
+}
+
 std::string StackTrace::ToString() const {
   std::stringstream stream;
 #if !defined(__UCLIBC__)

src/butil/debug/stack_trace.h

@@ -59,12 +59,20 @@ class BUTIL_EXPORT StackTrace {
 
   // Copying and assignment are allowed with the default functions.
 
-  ~StackTrace();
-
   // Gets an array of instruction pointer values. |*count| will be set to the
   // number of elements in the returned array.
   const void* const* Addresses(size_t* count) const;
 
+  // Gets the number of frames in the stack trace.
+  size_t FrameCount() const { return count_; }
+
+  // Copies the stack trace to |buffer|,
+  // where the size is min(max_nframes, frame_count()).
+  size_t CopyAddressTo(void** dest, size_t max_nframes) const;
+
+  // Whether if the given symbol is found in the stack trace.
+  bool FindSymbol(void* symbol) const;
+
   // Prints the stack trace to stderr.
   void Print() const;
 

src/butil/debug/stack_trace_posix.cc

@@ -45,7 +45,7 @@
 #include "butil/third_party/symbolize/symbolize.h"
 #endif
 
-extern int __attribute__((weak)) GetStackTrace(void** result, int max_depth, int skip_count);
+extern int BAIDU_WEAK GetStackTrace(void** result, int max_depth, int skip_count);
 
 namespace butil {
 namespace debug {
@@ -768,6 +768,24 @@ StackTrace::StackTrace(bool exclude_self) {
   }
 }
 
+bool StackTrace::FindSymbol(void* symbol) const {
+#if !defined(__UCLIBC__)
+  for (size_t i = 0; i < count_; ++i) {
+    uint64_t saddr;
+    // Subtract by one as return address of function may be in the next
+    // function when a function is annotated as noreturn.
+    void* address = static_cast<char*>(trace_[i]) - 1;
+    if (!google::SymbolizeAddress(address, &saddr)) {
+      continue;
+    }
+    if ((void*)saddr == symbol) {
+      return true;
+    }
+  }
+#endif
+  return false;
+}
+
 void StackTrace::Print() const {
   // NOTE: This code MUST be async-signal safe (it's used by in-process
   // stack dumping signal handler). NO malloc or stdio is allowed here.

src/butil/iobuf_profiler.cpp

@@ -24,7 +24,7 @@
 #include "butil/hash.h"
 #include <execinfo.h>
 
-extern int __attribute__((weak)) GetStackTrace(void** result, int max_depth, int skip_count);
+extern int BAIDU_WEAK GetStackTrace(void** result, int max_depth, int skip_count);
 
 namespace butil {
 
@@ -296,9 +296,7 @@ void SubmitIOBufSample(IOBuf::Block* block, int64_t ref) {
     auto sample = IOBufSample::New();
     sample->block = block;
     sample->count = ref;
-    sample->nframes = GetStackTrace(sample->stack,
-                                    arraysize(sample->stack),
-                                    0); // may lock
+    sample->nframes = GetStackTrace(sample->stack, arraysize(sample->stack), 0);
     IOBufProfiler::GetInstance()->Submit(sample);
 }
 

src/butil/object_pool.h

@@ -69,6 +69,11 @@ template <typename T> struct ObjectPoolValidator {
 
 namespace butil {
 
+// Whether if FreeChunk of LocalPool is empty.
+template <typename T> inline bool local_pool_free_empty() {
+    return ObjectPool<T>::singleton()->local_free_empty();
+}
+
 // Get an object typed |T|. The object should be cleared before usage.
 // NOTE: T must be default-constructible.
 template <typename T> inline T* get_object() {

src/butil/object_pool_inl.h

@@ -216,13 +216,25 @@ class BAIDU_CACHELINE_ALIGNMENT ObjectPool {
             return -1;
         }
 
+        inline bool free_empty() const {
+            return 0 == _cur_free.nfree;
+        }
+
     private:
         ObjectPool* _pool;
         Block* _cur_block;
         size_t _cur_block_index;
         FreeChunk _cur_free;
     };
 
+    inline bool local_free_empty() {
+        LocalPool* lp = get_or_new_local_pool();
+        if (BAIDU_LIKELY(lp != NULL)) {
+            return lp->free_empty();
+        }
+        return true;
+    }
+
     inline T* get_object() {
         LocalPool* lp = get_or_new_local_pool();
         if (BAIDU_LIKELY(lp != NULL)) {

src/butil/third_party/dynamic_annotations/dynamic_annotations.c

@@ -255,7 +255,7 @@ static int GetRunningOnValgrind(void) {
 }
 
 /* See the comments in dynamic_annotations.h */
-int __attribute__((weak)) RunningOnValgrind(void) {
+int DYNAMIC_ANNOTATIONS_ATTRIBUTE_WEAK RunningOnValgrind(void) {
   static volatile int running_on_valgrind = -1;
   /* C doesn't have thread-safe initialization of statics, and we
      don't want to depend on pthread_once here, so hack it. */