Work around the issue on fedora 26 that weak impl. of MallocExtension::instance…

Work around the issue on fedora 26 that weak impl. of MallocExtension::instance is not overriden by tcmalloc & Macro BAIDU_RPC_ENABLE_HEAP_PROFILER is not needed anymore

example/http_c++/Makefile

@@ -6,7 +6,7 @@ include $(BRPC_PATH)/config.mk
 # 2. Added -D__const__= : Avoid over-optimizations of TLS variables by GCC>=4.8
 CXXFLAGS = -std=c++0x -g -DNDEBUG -O2 -D__const__= -pipe -W -Wall -Werror -Wno-unused-parameter -fPIC -fno-omit-frame-pointer
 ifeq ($(NEED_GPERFTOOLS), 1)
-	CXXFLAGS+=-DBRPC_ENABLE_CPU_PROFILER -DBRPC_ENABLE_HEAP_PROFILER
+	CXXFLAGS+=-DBRPC_ENABLE_CPU_PROFILER
 endif
 HDRS+=$(BRPC_PATH)/output/include
 LIBS+=$(BRPC_PATH)/output/lib

example/multi_threaded_echo_c++/Makefile

@@ -6,7 +6,7 @@ include $(BRPC_PATH)/config.mk
 # 2. Added -D__const__= : Avoid over-optimizations of TLS variables by GCC>=4.8
 CXXFLAGS = -std=c++0x -g -DNDEBUG -O2 -D__const__= -pipe -W -Wall -Werror -Wno-unused-parameter -fPIC -fno-omit-frame-pointer
 ifeq ($(NEED_GPERFTOOLS), 1)
-	CXXFLAGS += -DBRPC_ENABLE_CPU_PROFILER -DBRPC_ENABLE_HEAP_PROFILER
+	CXXFLAGS += -DBRPC_ENABLE_CPU_PROFILER
 endif
 HDRS+=$(BRPC_PATH)/output/include
 LIBS+=$(BRPC_PATH)/output/lib

src/brpc/builtin/hotspots_service.cpp

@@ -40,7 +40,6 @@ void ContentionProfilerStop();
 namespace brpc {
 
 extern bool cpu_profiler_enabled;
-extern bool heap_profiler_enabled;
 
 DEFINE_int32(max_profiling_seconds, 300, "upper limit of running time of profilers");
 BRPC_VALIDATE_GFLAG(max_profiling_seconds, NonNegativeInteger);
@@ -661,9 +660,12 @@ static void DoProfiling(ProfilingType type,
         bthread::ContentionProfilerStop();
     } else if (type == PROFILING_HEAP) {
         MallocExtension* malloc_ext = MallocExtension::instance();
-        if (malloc_ext == NULL) {
-            os << "Heap profiler is not enabled"
-               << (use_html ? "</body></html>" : "\n");
+        if (malloc_ext == NULL || !has_TCMALLOC_SAMPLE_PARAMETER()) {
+            os << "Heap profiler is not enabled";
+            if (malloc_ext != NULL) {
+                os << " (no TCMALLOC_SAMPLE_PARAMETER in env)";
+            }
+            os << '.' << (use_html ? "</body></html>" : "\n");
             os.move_to(resp);
             cntl->http_response().set_status_code(HTTP_STATUS_FORBIDDEN);
             return NotifyWaiters(type, cntl, view);
@@ -681,7 +683,7 @@ static void DoProfiling(ProfilingType type,
     } else if (type == PROFILING_GROWTH) {
         MallocExtension* malloc_ext = MallocExtension::instance();
         if (malloc_ext == NULL) {
-            os << "Growth profiler is not enabled"
+            os << "Growth profiler is not enabled."
                << (use_html ? "</body></html>" : "\n");
             os.move_to(resp);
             cntl->http_response().set_status_code(HTTP_STATUS_FORBIDDEN);
@@ -726,20 +728,27 @@ static void StartProfiling(ProfilingType type,
     const bool use_html = UseHTML(cntl->http_request());
     butil::IOBufBuilder os;
     bool enabled = false;
+    const char* extra_desc = "";
     if (type == PROFILING_CPU) {
         enabled = cpu_profiler_enabled;
     } else if (type == PROFILING_CONTENTION) {
         enabled = true;
-    } else if (type == PROFILING_HEAP || type == PROFILING_GROWTH) {
-        enabled = heap_profiler_enabled;
+    } else if (type == PROFILING_HEAP) {
+        enabled = IsHeapProfilerEnabled();
+        if (enabled && !has_TCMALLOC_SAMPLE_PARAMETER()) {
+            enabled = false;
+            extra_desc = " (no TCMALLOC_SAMPLE_PARAMETER in env)";
+        }
+    } else if (type == PROFILING_GROWTH) {
+        enabled = IsHeapProfilerEnabled();
     }
     const char* const type_str = ProfilingType2String(type);
     
     if (!use_html) {
         if (!enabled) {
-            os << "Error: " << type_str << " profiler is not enabled yet.\n"
-                "To enable all profilers, link tcmalloc and define macros BRPC_ENABLE_CPU_PROFILER and BRPC_ENABLE_HEAP_PROFILER\n"
-                "Or read the docs: docs/cn/{cpu_profiler.md,heap_profiler.md}\n";
+            os << "Error: " << type_str << " profiler is not enabled."
+               << extra_desc << "\n"
+                "Read the docs: docs/cn/{cpu_profiler.md,heap_profiler.md}\n";
             os.move_to(cntl->response_attachment());
             cntl->http_response().set_status_code(HTTP_STATUS_FORBIDDEN);
             return;
@@ -980,10 +989,10 @@ static void StartProfiling(ProfilingType type,
     
     if (!enabled && view == NULL) {
         os << "<p><span style='color:red'>Error:</span> "
-           << type_str << " profiler is not enabled yet.</p>"
-            "<p>To enable all profilers, link tcmalloc and define macros BRPC_ENABLE_CPU_PROFILER and BRPC_ENABLE_HEAP_PROFILER"
-            "</p><p>Or read docs: <a href='https://github.com/brpc/brpc/blob/master/docs/cn/cpu_profiler.md'>cpu_profiler.md</a>"
-            " and <a href='https://github.com/brpc/brpc/blob/master/docs/cn/heap_profiler.md'>heap_profiler.md</a>"
+           << type_str << " profiler is not enabled." << extra_desc << "</p>"
+            "<p>To enable all profilers, link tcmalloc and define macros BRPC_ENABLE_CPU_PROFILER"
+            "</p><p>Or read docs: <a href='https://github.com/brpc/brpc/blob/master/docs/cn/cpu_profiler.md'>cpu_profiler</a>"
+            " and <a href='https://github.com/brpc/brpc/blob/master/docs/cn/heap_profiler.md'>heap_profiler</a>"
             "</p></body></html>";
         os.move_to(cntl->response_attachment());
         cntl->http_response().set_status_code(HTTP_STATUS_FORBIDDEN);

src/brpc/builtin/index_service.cpp

@@ -21,6 +21,7 @@
 #include "brpc/builtin/index_service.h"
 #include "brpc/builtin/status_service.h"
 #include "brpc/builtin/common.h"
+#include "brpc/details/tcmalloc_extension.h"
 
 namespace brpc {
 
@@ -36,7 +37,6 @@ DECLARE_bool(enable_threads_service);
 
 // Set in ProfilerLinker.
 bool cpu_profiler_enabled = false;
-bool heap_profiler_enabled = false;
 
 void IndexService::default_method(::google::protobuf::RpcController* controller,
                                   const IndexRequest*,
@@ -137,10 +137,10 @@ void IndexService::default_method(::google::protobuf::RpcController* controller,
            << Path("/hotspots/cpu", html_addr) << " : Profiling CPU"
            << (!cpu_profiler_enabled ? " (disabled)" : "") << NL
            << Path("/hotspots/heap", html_addr) << " : Profiling heap"
-           << (!heap_profiler_enabled ? " (disabled)" : "") << NL
+           << (!IsHeapProfilerEnabled() ? " (disabled)" : "") << NL
            << Path("/hotspots/growth", html_addr)
            << " : Profiling growth of heap"
-           << (!heap_profiler_enabled ? " (disabled)" : "") << NL;
+           << (!IsHeapProfilerEnabled() ? " (disabled)" : "") << NL;
     }
     os << "curl -H 'Content-Type: application/json' -d 'JSON' " << my_addr
        << "/ServiceName/MethodName : Call method by http+json" << NL

src/brpc/builtin/pprof_service.cpp

@@ -207,10 +207,14 @@ void PProfService::heap(
     ClosureGuard done_guard(done);
     Controller* cntl = static_cast<Controller*>(controller_base);
     MallocExtension* malloc_ext = MallocExtension::instance();
-    if (malloc_ext == NULL) {
-        cntl->SetFailed(ENOMETHOD, "%s, to enable heap profiler, check out "
-                        "docs/cn/heap_profiler.md",
-                        berror(ENOMETHOD));
+    if (malloc_ext == NULL || !has_TCMALLOC_SAMPLE_PARAMETER()) {
+        const char* extra_desc = "";
+        if (malloc_ext != NULL) {
+            extra_desc = " (no TCMALLOC_SAMPLE_PARAMETER in env)";
+        }
+        cntl->SetFailed(ENOMETHOD, "Heap profiler is not enabled%s,"
+                        "check out http://wiki.baidu.com/display/RPC",
+                        extra_desc);
         return;
     }
     // Log requester

src/brpc/details/profiler_linker.h

@@ -34,7 +34,6 @@ namespace brpc {
 
 // defined in src/brpc/builtin/index_service.cpp
 extern bool cpu_profiler_enabled;
-extern bool heap_profiler_enabled;
 
 // defined in src/brpc/controller.cpp
 extern int PROFILER_LINKER_DUMMY;
@@ -54,10 +53,6 @@ struct ProfilerLinker {
             ProfilerStart("this_function_should_never_run");
         }
 #endif
-    
-#if defined(BRPC_ENABLE_HEAP_PROFILER) || defined(BAIDU_RPC_ENABLE_HEAP_PROFILER)
-        heap_profiler_enabled = true;
-#endif
     }
 };
 

src/brpc/details/tcmalloc_extension.cpp

+#include <dlfcn.h>                               // dlsym
+#include <stdlib.h>                              // getenv
 #include "butil/compiler_specific.h"
 #include "brpc/details/tcmalloc_extension.h"
 
+namespace {
+typedef MallocExtension* (*GetInstanceFn)();
+static pthread_once_t g_get_instance_fn_once = PTHREAD_ONCE_INIT;
+static GetInstanceFn g_get_instance_fn = NULL;
+static void InitGetInstanceFn() {
+    g_get_instance_fn = (GetInstanceFn)dlsym(
+        RTLD_NEXT, "_ZN15MallocExtension8instanceEv");
+}
+} // namespace
+
 MallocExtension* BAIDU_WEAK MallocExtension::instance() {
+    // On fedora 26, this weak function is NOT overriden by the one in tcmalloc
+    // which is dynamically linked.The same issue can't be re-produced in
+    // Ubuntu and the exact cause is unknown yet. Using dlsym to get the
+    // function works around the issue right now. Note that we can't use dlsym
+    // to fully replace the weak-function mechanism since our code are generally
+    // not compiled with -rdynamic which writes symbols to the table that
+    // dlsym reads.
+    pthread_once(&g_get_instance_fn_once, InitGetInstanceFn);
+    if (g_get_instance_fn) {
+        return g_get_instance_fn();
+    }
     return NULL;
 }
+
+bool IsHeapProfilerEnabled() {
+    return MallocExtension::instance() != NULL;
+}
+
+static bool check_TCMALLOC_SAMPLE_PARAMETER() {
+    char* str = getenv("TCMALLOC_SAMPLE_PARAMETER");
+    if (str == NULL) {
+        return false;
+    }
+    char* endptr;
+    int val = strtol(str, &endptr, 10);
+    return (*endptr == '\0' && val > 0);
+}
+
+bool has_TCMALLOC_SAMPLE_PARAMETER() {
+    static bool val = check_TCMALLOC_SAMPLE_PARAMETER();
+    return val;
+}

src/brpc/details/tcmalloc_extension.h

@@ -311,3 +311,9 @@ class PERFTOOLS_DLL_DECL MallocExtension {
   // in the address space size.
   virtual void** ReadHeapGrowthStackTraces();
 };
+
+// True iff heap profiler is enabled.
+bool IsHeapProfilerEnabled();
+
+// True iff TCMALLOC_SAMPLE_PARAMETER is set in environment.
+bool has_TCMALLOC_SAMPLE_PARAMETER();

src/brpc/server.cpp

@@ -70,6 +70,7 @@
 #include "brpc/restful.h"
 #include "brpc/rtmp.h"
 #include "brpc/builtin/common.h"               // GetProgramName
+#include "brpc/details/tcmalloc_extension.h"
 
 inline std::ostream& operator<<(std::ostream& os, const timeval& tm) {
     const char old_fill = os.fill();
@@ -207,30 +208,15 @@ static void PrintSupportedCompressions(std::ostream& os, void*) {
     }
 }
 
-static bool check_TCMALLOC_SAMPLE_PARAMETER() {
-    char* str = getenv("TCMALLOC_SAMPLE_PARAMETER");
-    if (str == NULL) {
-        return false;
-    }
-    char* endptr;
-    int val = strtol(str, &endptr, 10);
-    return (*endptr == '\0' && val > 0);
-}
-
-static bool has_TCMALLOC_SAMPLE_PARAMETER() {
-    static bool val = check_TCMALLOC_SAMPLE_PARAMETER();
-    return val;
-}
-
 static void PrintEnabledProfilers(std::ostream& os, void*) {
     if (cpu_profiler_enabled) {
         os << "cpu ";
     }
-    if (heap_profiler_enabled) {
+    if (IsHeapProfilerEnabled()) {
         if (has_TCMALLOC_SAMPLE_PARAMETER()) {
             os << "heap ";
         } else {
-            os << "heap(lack of TCMALLOC_SAMPLE_PARAMETER) ";
+            os << "heap(no TCMALLOC_SAMPLE_PARAMETER in env) ";
         }
     }
     os << "contention";