SERVER-27727 Make threadName a native thread_local so debuggers can get to it

14 files changed, 105 insertions, 87 deletions

diff --git a/buildscripts/gdb/mongo.py b/buildscripts/gdb/mongo.py
index 57d046529c4..22420ef0ec2 100644
--- a/buildscripts/gdb/mongo.py
+++ b/buildscripts/gdb/mongo.py
@@ -391,6 +391,8 @@ MongoDBDumpLocks()
 class MongoDBUniqueStack(gdb.Command):
     """Print unique stack traces of all threads in current process"""
 
+    _HEADER_FORMAT = "Thread {gdb_thread_num}: {name} (Thread {pthread} (LWP {lwpid})):"
+
     def __init__(self):
         register_mongo_command(self, "mongodb-uniqstack", gdb.COMMAND_DATA)
 
@@ -411,57 +413,63 @@ class MongoDBUniqueStack(gdb.Command):
             if current_thread and current_thread.is_valid():
                 current_thread.switch()
 
+    def _get_current_thread_name(self):
+        fallback_name = '"%s"' % (gdb.selected_thread().name or '')
+        try:
+            # This goes through the pretty printer for StringData which adds "" around the name.
+            name = str(gdb.parse_and_eval("mongo::for_debuggers::threadName"))
+            if name == '""':
+                return fallback_name
+            return name
+        except gdb.error:
+            return fallback_name
+
     def _process_thread_stack(self, arg, stacks, thread):
         thread_info = {}  # thread dict to hold per thread data
-        thread_info['frames'] = []  # the complete backtrace per thread from gdb
-        thread_info['functions'] = []  # list of function names from frames
-
-        frame = gdb.newest_frame()
-        while frame:
-            thread_info['functions'].append(frame.name())
-            frame = frame.older()
-
-        thread_info['functions'] = tuple(thread_info['functions'])
-        if thread_info['functions'] in stacks:
-            stacks[thread_info['functions']]['tids'].append(thread.num)
-            return
-
         thread_info['pthread'] = get_thread_id()
-        (_, thread_lwpid, thread_tid) = thread.ptid
+        thread_info['gdb_thread_num'] = thread.num
+        thread_info['lwpid'] = thread.ptid[1]
+        thread_info['name'] = self._get_current_thread_name()
+
         if sys.platform.startswith("linux"):
-            header_format = "Thread {gdb_thread_num} (Thread 0x{pthread:x} (LWP {lwpid}))"
+            header_format = "Thread {gdb_thread_num}: {name} (Thread 0x{pthread:x} (LWP {lwpid}))"
         elif sys.platform.startswith("sunos"):
-            if thread_tid != 0 and thread_lwpid != 0:
-                header_format = "Thread {gdb_thread_num} (Thread {pthread} (LWP {lwpid}))"
-            elif thread_lwpid != 0:
-                header_format = "Thread {gdb_thread_num} (LWP {lwpid})"
+            (_, _, thread_tid) = thread.ptid
+            if thread_tid != 0 and thread_info['lwpid'] != 0:
+                header_format = "Thread {gdb_thread_num}: {name} (Thread {pthread} (LWP {lwpid}))"
+            elif thread_info['lwpid'] != 0:
+                header_format = "Thread {gdb_thread_num}: {name} (LWP {lwpid})"
             else:
-                header_format = "Thread {gdb_thread_num} (Thread {pthread})"
+                header_format = "Thread {gdb_thread_num}: {name} (Thread {pthread})"
         else:
             raise ValueError("Unsupported platform: {}".format(sys.platform))
-        thread_info['header'] = header_format.format(gdb_thread_num=thread.num,
-                                                     pthread=thread_info['pthread'],
-                                                     lwpid=thread_lwpid)
-        try:
-            thread_info['frames'] = gdb.execute(arg, to_string=True).rstrip()
-        except gdb.error as err:
-            raise gdb.GdbError("{} {}".format(thread_info['header'], err))
-        else:
-            thread_info['tids'] = []
-            thread_info['tids'].append(thread.num)
-            stacks[thread_info['functions']] = thread_info
+        thread_info['header'] = header_format.format(**thread_info)
+
+        functions = [] # list of function names from frames
+        frame = gdb.newest_frame()
+        while frame:
+            functions.append(frame.name())
+            frame = frame.older()
+        functions = tuple(functions) # tuples are hashable, lists aren't.
+
+        unique = stacks.setdefault(functions, {'threads': []})
+        unique['threads'].append(thread_info)
+        if 'output' not in unique:
+            try:
+                unique['output'] = gdb.execute(arg, to_string=True).rstrip()
+            except gdb.error as err:
+                raise gdb.GdbError("{} {}".format(thread_info['header'], err))
 
     def _dump_unique_stacks(self, stacks):
         def first_tid(stack):
-            return stack['tids'][0]
+            return stack['threads'][0]['gdb_thread_num']
 
         for stack in sorted(stacks.values(), key=first_tid, reverse=True):
-            print(stack['header'])
-            if len(stack['tids']) > 1:
-                print("{} duplicate thread(s):".format(len(stack['tids']) - 1), end=' ')
-                print(", ".join((str(tid) for tid in stack['tids'][1:])))
-            print(stack['frames'])
-            print()  # leave extra blank line after each thread stack
+            for i, thread in enumerate(stack['threads']):
+                prefix = '' if i == 0 else 'Duplicate '
+                print(prefix + thread['header'])
+            print(stack['output'])
+            print() # leave extra blank line after each thread stack
 
 # Register command
 MongoDBUniqueStack()
diff --git a/etc/lsan.suppressions b/etc/lsan.suppressions
index 8d0eaea9296..96c83f6ea33 100644
--- a/etc/lsan.suppressions
+++ b/etc/lsan.suppressions
@@ -1,8 +1,4 @@
 # Client objects are leaked in threads that are never terminated
 leak:mongo::Client::Client
 
-# Thread names leak from threads that are never terminated.
-leak:mongo::setThreadName
-leak:mongo::getThreadName
-
 leak:glob64
diff --git a/src/mongo/db/client.cpp b/src/mongo/db/client.cpp
index 339fcdc9d9d..9fdb3ce6cf6 100644
--- a/src/mongo/db/client.cpp
+++ b/src/mongo/db/client.cpp
@@ -53,21 +53,21 @@ namespace mongo {
 TSP_DECLARE(ServiceContext::UniqueClient, currentClient)
 TSP_DEFINE(ServiceContext::UniqueClient, currentClient)
 
-void Client::initThreadIfNotAlready(const char* desc) {
+void Client::initThreadIfNotAlready(StringData desc) {
     if (currentClient.getMake()->get())
         return;
     initThread(desc);
 }
 
 void Client::initThreadIfNotAlready() {
-    initThreadIfNotAlready(getThreadName().c_str());
+    initThreadIfNotAlready(getThreadName());
 }
 
-void Client::initThread(const char* desc, transport::SessionHandle session) {
+void Client::initThread(StringData desc, transport::SessionHandle session) {
     initThread(desc, getGlobalServiceContext(), std::move(session));
 }
 
-void Client::initThread(const char* desc,
+void Client::initThread(StringData desc,
                         ServiceContext* service,
                         transport::SessionHandle session) {
     invariant(currentClient.getMake()->get() == nullptr);
@@ -76,10 +76,10 @@ void Client::initThread(const char* desc,
     if (session) {
         fullDesc = str::stream() << desc << session->id();
     } else {
-        fullDesc = desc;
+        fullDesc = desc.toString();
     }
 
-    setThreadName(fullDesc.c_str());
+    setThreadName(fullDesc);
 
     // Create the client obj, attach to thread
     *currentClient.get() = service->makeClient(fullDesc, std::move(session));
diff --git a/src/mongo/db/client.h b/src/mongo/db/client.h
index 32d8de0fc04..e37dd6d7afa 100644
--- a/src/mongo/db/client.h
+++ b/src/mongo/db/client.h
@@ -71,8 +71,8 @@ public:
      *
      * If provided, session's ref count will be bumped by this Client.
      */
-    static void initThread(const char* desc, transport::SessionHandle session = nullptr);
-    static void initThread(const char* desc,
+    static void initThread(StringData desc, transport::SessionHandle session = nullptr);
+    static void initThread(StringData desc,
                            ServiceContext* serviceContext,
                            transport::SessionHandle session);
 
@@ -116,7 +116,7 @@ public:
      * Inits a thread if that thread has not already been init'd, setting the thread name to
      * "desc".
      */
-    static void initThreadIfNotAlready(const char* desc);
+    static void initThreadIfNotAlready(StringData desc);
 
     /**
      * Inits a thread if that thread has not already been init'd, using the existing thread name
diff --git a/src/mongo/db/s/balancer/migration_manager.cpp b/src/mongo/db/s/balancer/migration_manager.cpp
index 9c70e8458ba..eb47bb227c8 100644
--- a/src/mongo/db/s/balancer/migration_manager.cpp
+++ b/src/mongo/db/s/balancer/migration_manager.cpp
@@ -520,7 +520,7 @@ void MigrationManager::_schedule_inlock(OperationContext* opCtx,
         executor->scheduleRemoteCommand(
             remoteRequest,
             [this, itMigration](const executor::TaskExecutor::RemoteCommandCallbackArgs& args) {
-                Client::initThread(getThreadName().c_str());
+                Client::initThread(getThreadName());
                 ON_BLOCK_EXIT([&] { Client::destroy(); });
                 auto opCtx = cc().makeOperationContext();
 
diff --git a/src/mongo/db/s/collection_range_deleter.cpp b/src/mongo/db/s/collection_range_deleter.cpp
index 2349eb4a3bd..be37e510a9d 100644
--- a/src/mongo/db/s/collection_range_deleter.cpp
+++ b/src/mongo/db/s/collection_range_deleter.cpp
@@ -73,7 +73,7 @@ const WriteConcernOptions kMajorityWriteConcern(WriteConcernOptions::kMajority,
 CollectionRangeDeleter::CollectionRangeDeleter(NamespaceString nss) : _nss(std::move(nss)) {}
 
 void CollectionRangeDeleter::run() {
-    Client::initThread(getThreadName().c_str());
+    Client::initThread(getThreadName());
     ON_BLOCK_EXIT([&] { Client::destroy(); });
     auto opCtx = cc().makeOperationContext().get();
 
diff --git a/src/mongo/db/service_context_d_test_fixture.cpp b/src/mongo/db/service_context_d_test_fixture.cpp
index be62c952d22..3ea54ecf2d6 100644
--- a/src/mongo/db/service_context_d_test_fixture.cpp
+++ b/src/mongo/db/service_context_d_test_fixture.cpp
@@ -52,7 +52,7 @@
 namespace mongo {
 
 void ServiceContextMongoDTest::setUp() {
-    Client::initThread(getThreadName().c_str());
+    Client::initThread(getThreadName());
     ServiceContext* serviceContext = getServiceContext();
 
     std::array<std::uint8_t, 20> tempKey = {};
diff --git a/src/mongo/dbtests/jstests.cpp b/src/mongo/dbtests/jstests.cpp
index 8569720e242..8ba67219d91 100644
--- a/src/mongo/dbtests/jstests.cpp
+++ b/src/mongo/dbtests/jstests.cpp
@@ -160,7 +160,7 @@ class LogRecordingScope {
 public:
     LogRecordingScope()
         : _logged(false),
-          _threadName(mongo::getThreadName()),
+          _threadName(mongo::getThreadName().toString()),
           _handle(mongo::logger::globalLogDomain()->attachAppender(
               mongo::logger::MessageLogDomain::AppenderAutoPtr(new Tee(this)))) {}
     ~LogRecordingScope() {
diff --git a/src/mongo/logger/logstream_builder.cpp b/src/mongo/logger/logstream_builder.cpp
index 69d3e151a47..af055be3120 100644
--- a/src/mongo/logger/logstream_builder.cpp
+++ b/src/mongo/logger/logstream_builder.cpp
@@ -73,25 +73,24 @@ struct ThreadOstreamCacheFinalizer {
 namespace logger {
 
 LogstreamBuilder::LogstreamBuilder(MessageLogDomain* domain,
-                                   std::string contextName,
+                                   StringData contextName,
                                    LogSeverity severity)
-    : LogstreamBuilder(
-          domain, std::move(contextName), std::move(severity), LogComponent::kDefault) {}
+    : LogstreamBuilder(domain, contextName, std::move(severity), LogComponent::kDefault) {}
 
 LogstreamBuilder::LogstreamBuilder(MessageLogDomain* domain,
-                                   std::string contextName,
+                                   StringData contextName,
                                    LogSeverity severity,
                                    LogComponent component)
     : _domain(domain),
-      _contextName(std::move(contextName)),
+      _contextName(contextName.toString()),
       _severity(std::move(severity)),
       _component(std::move(component)),
       _tee(nullptr) {}
 
 LogstreamBuilder::LogstreamBuilder(logger::MessageLogDomain* domain,
-                                   const std::string& contextName,
+                                   StringData contextName,
                                    LabeledLevel labeledLevel)
-    : LogstreamBuilder(domain, std::move(contextName), static_cast<LogSeverity>(labeledLevel)) {
+    : LogstreamBuilder(domain, contextName, static_cast<LogSeverity>(labeledLevel)) {
     setBaseMessage(labeledLevel.getLabel());
 }
 
diff --git a/src/mongo/logger/logstream_builder.h b/src/mongo/logger/logstream_builder.h
index 001175e6e1c..1dac8462564 100644
--- a/src/mongo/logger/logstream_builder.h
+++ b/src/mongo/logger/logstream_builder.h
@@ -65,7 +65,7 @@ public:
      * "contextName" is a short name of the thread or other context.
      * "severity" is the logging severity of the message.
      */
-    LogstreamBuilder(MessageLogDomain* domain, std::string contextName, LogSeverity severity);
+    LogstreamBuilder(MessageLogDomain* domain, StringData contextName, LogSeverity severity);
 
     /**
      * Construct a LogstreamBuilder that writes to "domain" on destruction.
@@ -75,16 +75,14 @@ public:
      * "component" is the primary log component of the message.
      */
     LogstreamBuilder(MessageLogDomain* domain,
-                     std::string contextName,
+                     StringData contextName,
                      LogSeverity severity,
                      LogComponent component);
 
     /**
      * Deprecated.
      */
-    LogstreamBuilder(MessageLogDomain* domain,
-                     const std::string& contextName,
-                     LabeledLevel labeledLevel);
+    LogstreamBuilder(MessageLogDomain* domain, StringData contextName, LabeledLevel labeledLevel);
 
     LogstreamBuilder(LogstreamBuilder&& other) = default;
     LogstreamBuilder& operator=(LogstreamBuilder&& other) = default;
diff --git a/src/mongo/transport/service_entry_point_utils.cpp b/src/mongo/transport/service_entry_point_utils.cpp
index 5ceec054fc7..a0f3dedb285 100644
--- a/src/mongo/transport/service_entry_point_utils.cpp
+++ b/src/mongo/transport/service_entry_point_utils.cpp
@@ -72,7 +72,7 @@ void* runFunc(void* ptr) {
 
     auto tl = ctx->session->getTransportLayer();
     Client::initThread("conn", ctx->session);
-    setThreadName(std::string(str::stream() << "conn" << ctx->session->id()));
+    setThreadName(str::stream() << "conn" << ctx->session->id());
 
     try {
         ctx->task(ctx->session);
diff --git a/src/mongo/util/background.cpp b/src/mongo/util/background.cpp
index d6cf34abdc0..6e3d27501c6 100644
--- a/src/mongo/util/background.cpp
+++ b/src/mongo/util/background.cpp
@@ -142,7 +142,7 @@ BackgroundJob::~BackgroundJob() {}
 void BackgroundJob::jobBody() {
     const string threadName = name();
     if (!threadName.empty()) {
-        setThreadName(threadName.c_str());
+        setThreadName(threadName);
     }
 
     LOG(1) << "BackgroundJob starting: " << threadName;
diff --git a/src/mongo/util/concurrency/thread_name.cpp b/src/mongo/util/concurrency/thread_name.cpp
index 78122ed213f..d2917fd5c02 100644
--- a/src/mongo/util/concurrency/thread_name.cpp
+++ b/src/mongo/util/concurrency/thread_name.cpp
@@ -47,6 +47,7 @@
 #include "mongo/base/init.h"
 #include "mongo/config.h"
 #include "mongo/platform/atomic_word.h"
+#include "mongo/util/concurrency/threadlocal.h"
 #include "mongo/util/log.h"
 #include "mongo/util/mongoutils/str.h"
 
@@ -86,7 +87,6 @@ void setWindowsThreadName(DWORD dwThreadID, const char* threadName) {
 }
 #endif
 
-boost::thread_specific_ptr<std::string> threadName;
 AtomicInt64 nextUnnamedThreadId{1};
 
 // It is unsafe to access threadName before its dynamic initialization has completed. Use
@@ -102,20 +102,37 @@ MONGO_INITIALIZER(ThreadNameInitializer)(InitializerContext*) {
     return Status::OK();
 }
 
+// TODO consider making threadName std::string and removing the size limit once we get real
+// thread_local.
+constexpr size_t kMaxThreadNameSize = 63;
+MONGO_TRIVIALLY_CONSTRUCTIBLE_THREAD_LOCAL char threadNameStorage[kMaxThreadNameSize + 1];
+
 }  // namespace
 
+namespace for_debuggers {
+// This needs external linkage to ensure that debuggers can use it.
+MONGO_TRIVIALLY_CONSTRUCTIBLE_THREAD_LOCAL StringData threadName;
+}
+using for_debuggers::threadName;
+
 void setThreadName(StringData name) {
     invariant(mongoInitializersHaveRun);
-    threadName.reset(new string(name.toString()));
+    if (name.size() > kMaxThreadNameSize) {
+        // Truncate unreasonably long thread names.
+        name = name.substr(0, kMaxThreadNameSize);
+    }
+    name.copyTo(threadNameStorage, /*null terminate=*/true);
+    threadName = StringData(threadNameStorage, name.size());
 
 #if defined(_WIN32)
     // Naming should not be expensive compared to thread creation and connection set up, but if
     // testing shows otherwise we should make this depend on DEBUG again.
-    setWindowsThreadName(GetCurrentThreadId(), threadName->c_str());
+    setWindowsThreadName(GetCurrentThreadId(), threadName.rawData());
 #elif defined(__APPLE__)
     // Maximum thread name length on OS X is MAXTHREADNAMESIZE (64 characters). This assumes
     // OS X 10.6 or later.
-    int error = pthread_setname_np(threadName->substr(0, MAXTHREADNAMESIZE - 1).c_str());
+    MONGO_STATIC_ASSERT(MAXTHREADNAMESIZE >= kMaxThreadNameSize + 1);
+    int error = pthread_setname_np(threadName.rawData());
     if (error) {
         log() << "Ignoring error from setting thread name: " << errnoWithDescription(error);
     }
@@ -126,15 +143,15 @@ void setThreadName(StringData name) {
     if (getpid() != syscall(SYS_gettid)) {
         //  Maximum thread name length supported on Linux is 16 including the null terminator.
         //  Ideally we use short and descriptive thread names that fit: this helps for log
-        //  readibility as well. Still, as the limit is so low and a few current names exceed the
+        //  readability as well. Still, as the limit is so low and a few current names exceed the
         //  limit, it's best to shorten long names.
         int error = 0;
-        if (threadName->size() > 15) {
-            std::string shortName =
-                threadName->substr(0, 7) + '.' + threadName->substr(threadName->size() - 7);
+        if (threadName.size() > 15) {
+            std::string shortName = str::stream() << threadName.substr(0, 7) << '.'
+                                                  << threadName.substr(threadName.size() - 7);
             error = pthread_setname_np(pthread_self(), shortName.c_str());
         } else {
-            error = pthread_setname_np(pthread_self(), threadName->c_str());
+            error = pthread_setname_np(pthread_self(), threadName.rawData());
         }
 
         if (error) {
@@ -144,7 +161,7 @@ void setThreadName(StringData name) {
 #endif
 }
 
-const string& getThreadName() {
+StringData getThreadName() {
     if (MONGO_unlikely(!mongoInitializersHaveRun)) {
         // 'getThreadName' has been called before dynamic initialization for this
         // translation unit has completed, so return a fallback value rather than accessing
@@ -154,11 +171,10 @@ const string& getThreadName() {
         return kFallback;
     }
 
-    std::string* s;
-    while (!(s = threadName.get())) {
-        setThreadName(std::string(str::stream() << "thread" << nextUnnamedThreadId.fetchAndAdd(1)));
+    if (threadName.empty()) {
+        setThreadName(str::stream() << "thread" << nextUnnamedThreadId.fetchAndAdd(1));
     }
-    return *s;
+    return threadName;
 }
 
 }  // namespace mongo
diff --git a/src/mongo/util/concurrency/thread_name.h b/src/mongo/util/concurrency/thread_name.h
index 3e05b0529d6..faececb6e8b 100644
--- a/src/mongo/util/concurrency/thread_name.h
+++ b/src/mongo/util/concurrency/thread_name.h
@@ -34,14 +34,15 @@
 namespace mongo {
 
 /**
- * Sets the name of the current thread to "name".
+ * Sets the name of the current thread.
  */
 void setThreadName(StringData name);
 
 /**
- * Retrieves the name of the current thread, as previously set, or "" if no name was previously
- * set.
+ * Retrieves the name of the current thread, as previously set, or "thread#" if no name was
+ * previously set. The returned StringData is always null terminated so it is safe to pass to APIs
+ * that expect c-strings.
  */
-const std::string& getThreadName();
+StringData getThreadName();
 
 }  // namespace mongo