From f792240c4419b7a3e7dfef652ee61e0996c3e749 Mon Sep 17 00:00:00 2001 From: "Patrick M. Niedzielski" Date: Wed, 16 Apr 2025 13:23:47 -0400 Subject: [PATCH 1/3] Fix: Use `bslmt::ThreadUtil::setThreadName` This patch replaces the manual implementation of `bmqsys::ThreadUtil::setCurrentThreadName` with an implementation using BDE, which supports more platforms. For the moment, we keep `bmqsys::ThreadUtil::k_SUPPORT_THREAD_NAME` only being true on Linux, as there appear to be other places this constant is used. Signed-off-by: Patrick M. Niedzielski --- src/groups/bmq/bmqsys/bmqsys_threadutil.cpp | 42 +++------------------ src/groups/bmq/bmqsys/bmqsys_threadutil.h | 20 +++++----- 2 files changed, 15 insertions(+), 47 deletions(-) diff --git a/src/groups/bmq/bmqsys/bmqsys_threadutil.cpp b/src/groups/bmq/bmqsys/bmqsys_threadutil.cpp index 4270271baa..25f8890db1 100644 --- a/src/groups/bmq/bmqsys/bmqsys_threadutil.cpp +++ b/src/groups/bmq/bmqsys/bmqsys_threadutil.cpp @@ -26,15 +26,11 @@ #include #include #include +#include #include #include #include -// Linux -#if defined(BSLS_PLATFORM_OS_LINUX) -#include -#endif - namespace BloombergLP { namespace bmqsys { @@ -55,20 +51,14 @@ bslmt::ThreadAttributes ThreadUtil::defaultAttributes() // LINUX // ----- #if defined(BSLS_PLATFORM_OS_LINUX) - const bool ThreadUtil::k_SUPPORT_THREAD_NAME = true; +#else +const bool ThreadUtil::k_SUPPORT_THREAD_NAME = false; +#endif void ThreadUtil::setCurrentThreadName(const bsl::string& value) { - int rc = prctl(PR_SET_NAME, value.c_str(), 0, 0, 0); - // We should use 'modern' APIs: pthread_setname_no(pthread_self()). But - // Bloomberg is a bit old; API was added in glibc 2.12, and we have 2.5. - if (rc != 0) { - BALL_LOG_SET_CATEGORY(k_LOG_CATEGORY); - BALL_LOG_ERROR << "Failed to set thread name " << "[name: '" << value - << "'" << ", rc: " << rc << ", strerr: '" - << bsl::strerror(rc) << "']"; - } + bslmt::ThreadUtil::setThreadName(value); } void ThreadUtil::setCurrentThreadNameOnce(const bsl::string& value) @@ -87,27 +77,5 @@ void ThreadUtil::setCurrentThreadNameOnce(const bsl::string& value) } } -// UNSUPPORTED_PLATFORMS -// --------------------- -#else - -const bool ThreadUtil::k_SUPPORT_THREAD_NAME = false; - -void ThreadUtil::setCurrentThreadName( - BSLS_ANNOTATION_UNUSED const bsl::string& value) -{ - // NOT AVAILABLE - - static_cast(k_LOG_CATEGORY); // suppress unused variable warning -} - -void ThreadUtil::setCurrentThreadNameOnce( - BSLS_ANNOTATION_UNUSED const bsl::string& value) -{ - // NOT AVAILABLE -} - -#endif - } // close package namespace } // close enterprise namespace diff --git a/src/groups/bmq/bmqsys/bmqsys_threadutil.h b/src/groups/bmq/bmqsys/bmqsys_threadutil.h index d097a0e3cf..1faeb3f4be 100644 --- a/src/groups/bmq/bmqsys/bmqsys_threadutil.h +++ b/src/groups/bmq/bmqsys/bmqsys_threadutil.h @@ -59,22 +59,22 @@ struct ThreadUtil { /// thread parameter values set for the local operating system. static bslmt::ThreadAttributes defaultAttributes(); - /// Set the name of the current thread to the specified `value`. This - /// method is a no-op if `k_SUPPORT_THREAD_NAME` is false. + /// Set the name of the current thread to the specified `value`, truncated + /// to a length of 15 bytes. /// /// PLATFORM NOTE: - /// - this functionality is only supported on LINUX, and the name can - /// be up to 15 characters. + /// - On platforms other than Linux, Solaris, Darwin, and Windows, this + /// method has no effect. static void setCurrentThreadName(const bsl::string& value); - /// Set the name of the current thread to the specified `value`. This - /// method is a no-op if `k_SUPPORT_THREAD_NAME` is false. Unlike - /// `setCurrentThreadName`, this method uses a thread local variable to - /// ensure this is done only once per thread. + /// Set the name of the current thread to the specified `value`, truncated + /// to a length of 15 bytes. Unlike `setCurrentThreadName`, this method + /// uses a thread local variable to ensure this is done only once per + /// thread. /// /// PLATFORM NOTE: - /// - this functionality is only supported on LINUX, and the name can - /// be up to 15 characters. + /// - On platforms other than Linux, Solaris, Darwin, and Windows, this + /// method has no effect. static void setCurrentThreadNameOnce(const bsl::string& value); }; From cc46683d9e0f9baf5297651e8e68da745ede6195 Mon Sep 17 00:00:00 2001 From: "Patrick M. Niedzielski" Date: Wed, 16 Apr 2025 13:32:03 -0400 Subject: [PATCH 2/3] Fix: Remove all uses of `bmqsys::ThreadUtil::k_SUPPORT_THREAD_NAME` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contracts of `bmqsys::ThreadUtil::setCurrentThreadName` and `bmqsys::ThreadUtil::setCurrentThreadNameOnce` are wide; they may be called on any platform, and they have no side effects on platforms where setting a thread name is unsupported. This means we do not need to guard calls to them by checking a constant that is only true on platforms where setting a thread name is supported. This patch removes all such guards, which removes all uses of this constant. In most cases, the use of this constant is in an `if` statement that obviously wraps some call to `bmqsys::ThreadUtil::setCurrentThreadName` or `bmqsys::ThreadUtil::setCurrentThreadNameOnce`. The only less obvious case is in `m_bmqbrkr_task`’s mtrap support, where this constant guards a write of the command `k_MTRAP_SET_THREADNAME` to the mtrap pipe. The command is handled by `Task::onControlMessage` in the same file, which in turn calls `bmqsys::ThreadUtil::setCurrentThreadName`. Presumably, this is where the `if` guard should have been, but now it is safe to remove the guard on the write. Signed-off-by: Patrick M. Niedzielski --- src/applications/bmqbrkr/m_bmqbrkr_task.cpp | 15 +++++--------- .../bmq/bmqimp/bmqimp_brokersession.cpp | 4 +--- src/groups/mqb/mqba/mqba_dispatcher.cpp | 20 +++++++++---------- src/groups/mqb/mqbnet/mqbnet_elector.cpp | 14 ++++++------- .../mqb/mqbnet/mqbnet_tcpsessionfactory.cpp | 4 +--- .../mqb/mqbstat/mqbstat_statcontroller.cpp | 10 ++++------ 6 files changed, 26 insertions(+), 41 deletions(-) diff --git a/src/applications/bmqbrkr/m_bmqbrkr_task.cpp b/src/applications/bmqbrkr/m_bmqbrkr_task.cpp index ea8fb712f8..f7205645ea 100644 --- a/src/applications/bmqbrkr/m_bmqbrkr_task.cpp +++ b/src/applications/bmqbrkr/m_bmqbrkr_task.cpp @@ -279,12 +279,10 @@ int Task::initialize(bsl::ostream& errorDescription) return rc_SCHEDULER_START_FAILED; // RETURN } - if (bmqsys::ThreadUtil::k_SUPPORT_THREAD_NAME) { - d_scheduler.scheduleEvent( - bsls::TimeInterval(0, 0), // now - bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, - "bmqSchedTask")); - } + d_scheduler.scheduleEvent( + bsls::TimeInterval(0, 0), // now + bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, + "bmqSchedTask")); // ------------- // LogController @@ -344,10 +342,7 @@ int Task::initialize(bsl::ostream& errorDescription) return rc_CONTROLCHANNEL_START_FAILED; // RETURN } - if (bmqsys::ThreadUtil::k_SUPPORT_THREAD_NAME) { - bdls::PipeUtil::send(pipePath, - bsl::string(k_MTRAP_SET_THREADNAME) + "\n"); - } + bdls::PipeUtil::send(pipePath, bsl::string(k_MTRAP_SET_THREADNAME) + "\n"); // ------------------- // M-Trap registration diff --git a/src/groups/bmq/bmqimp/bmqimp_brokersession.cpp b/src/groups/bmq/bmqimp/bmqimp_brokersession.cpp index 835614901b..14883fd00c 100644 --- a/src/groups/bmq/bmqimp/bmqimp_brokersession.cpp +++ b/src/groups/bmq/bmqimp/bmqimp_brokersession.cpp @@ -5850,9 +5850,7 @@ void BrokerSession::setChannel(const bsl::shared_ptr& channel) { // executed by the *IO* thread - if (bmqsys::ThreadUtil::k_SUPPORT_THREAD_NAME) { - bmqsys::ThreadUtil::setCurrentThreadNameOnce("bmqTCPIO"); - } + bmqsys::ThreadUtil::setCurrentThreadNameOnce("bmqTCPIO"); if (channel) { // We are now connected to bmqbrkr BALL_LOG_INFO << "Channel is CREATED [host: " << channel->peerUri() diff --git a/src/groups/mqb/mqba/mqba_dispatcher.cpp b/src/groups/mqb/mqba/mqba_dispatcher.cpp index 2d4b00986b..f9560e53e2 100644 --- a/src/groups/mqb/mqba/mqba_dispatcher.cpp +++ b/src/groups/mqb/mqba/mqba_dispatcher.cpp @@ -522,17 +522,15 @@ int Dispatcher::start(bsl::ostream& errorDescription) return rc; // RETURN } - if (bmqsys::ThreadUtil::k_SUPPORT_THREAD_NAME) { - execute(bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, - "bmqDispSession"), - mqbi::DispatcherClientType::e_SESSION); - execute(bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, - "bmqDispQueue"), - mqbi::DispatcherClientType::e_QUEUE); - execute(bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, - "bmqDispCluster"), - mqbi::DispatcherClientType::e_CLUSTER); - } + execute(bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, + "bmqDispSession"), + mqbi::DispatcherClientType::e_SESSION); + execute(bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, + "bmqDispQueue"), + mqbi::DispatcherClientType::e_QUEUE); + execute(bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, + "bmqDispCluster"), + mqbi::DispatcherClientType::e_CLUSTER); d_isStarted = true; diff --git a/src/groups/mqb/mqbnet/mqbnet_elector.cpp b/src/groups/mqb/mqbnet/mqbnet_elector.cpp index 9d25d1f64a..8f0b6722b6 100644 --- a/src/groups/mqb/mqbnet/mqbnet_elector.cpp +++ b/src/groups/mqb/mqbnet/mqbnet_elector.cpp @@ -2234,15 +2234,13 @@ Elector::Elector(mqbcfg::ElectorConfig& config, d_state.setTerm(initialTerm); - if (bmqsys::ThreadUtil::k_SUPPORT_THREAD_NAME) { - // Per scheduler's contract, it's ok to schedule events before its - // started. + // Per scheduler's contract, it's ok to schedule events before its + // started. - d_scheduler.scheduleEvent( - bsls::TimeInterval(0, 0), // now - bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, - "bmqSchedElec")); - } + d_scheduler.scheduleEvent( + bsls::TimeInterval(0, 0), // now + bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, + "bmqSchedElec")); } Elector::~Elector() diff --git a/src/groups/mqb/mqbnet/mqbnet_tcpsessionfactory.cpp b/src/groups/mqb/mqbnet/mqbnet_tcpsessionfactory.cpp index 2220e5c9b4..2e7498f8dd 100644 --- a/src/groups/mqb/mqbnet/mqbnet_tcpsessionfactory.cpp +++ b/src/groups/mqb/mqbnet/mqbnet_tcpsessionfactory.cpp @@ -656,9 +656,7 @@ void TCPSessionFactory::channelStateCallback( // This is an infrequent enough operation (compared to a 'readCb') that it // is fine to do this here (since we have no other ways to // proactively-execute code in the IO threads created by the channelPool). - if (bmqsys::ThreadUtil::k_SUPPORT_THREAD_NAME) { - bmqsys::ThreadUtil::setCurrentThreadNameOnce(d_threadName); - } + bmqsys::ThreadUtil::setCurrentThreadNameOnce(d_threadName); BALL_LOG_TRACE << "TCPSessionFactory '" << d_config.name() << "': channelStateCallback [event: " << event diff --git a/src/groups/mqb/mqbstat/mqbstat_statcontroller.cpp b/src/groups/mqb/mqbstat/mqbstat_statcontroller.cpp index 97677494b7..7614879863 100644 --- a/src/groups/mqb/mqbstat/mqbstat_statcontroller.cpp +++ b/src/groups/mqb/mqbstat/mqbstat_statcontroller.cpp @@ -725,12 +725,10 @@ int StatController::start(bsl::ostream& errorDescription) return -2; // RETURN } - if (bmqsys::ThreadUtil::k_SUPPORT_THREAD_NAME) { - d_scheduler_mp->scheduleEvent( - bsls::TimeInterval(0), // execute as soon as possible - bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, - "bmqSchedStat")); - } + d_scheduler_mp->scheduleEvent( + bsls::TimeInterval(0), // execute as soon as possible + bdlf::BindUtil::bind(&bmqsys::ThreadUtil::setCurrentThreadName, + "bmqSchedStat")); // Create and start the system stat monitor. The SystemStats are used in // the dashboard screen, stat consumers, stats printer, ... So we need to From 21f316d7af83c088c548daa86f9430de030dc4a9 Mon Sep 17 00:00:00 2001 From: "Patrick M. Niedzielski" Date: Wed, 16 Apr 2025 13:41:51 -0400 Subject: [PATCH 3/3] Fix: Remove unused constant `bmqsys::ThreadUtil::k_SUPPORT_THREAD_NAME` With the previous commit, this constant is no longer used in BlazingMQ. Because it is not part of our public interface, it is safe to remove this constant. This patch removes it. Signed-off-by: Patrick M. Niedzielski --- src/groups/bmq/bmqsys/bmqsys_threadutil.cpp | 8 -------- src/groups/bmq/bmqsys/bmqsys_threadutil.h | 6 ------ 2 files changed, 14 deletions(-) diff --git a/src/groups/bmq/bmqsys/bmqsys_threadutil.cpp b/src/groups/bmq/bmqsys/bmqsys_threadutil.cpp index 25f8890db1..a4e01e3392 100644 --- a/src/groups/bmq/bmqsys/bmqsys_threadutil.cpp +++ b/src/groups/bmq/bmqsys/bmqsys_threadutil.cpp @@ -48,14 +48,6 @@ bslmt::ThreadAttributes ThreadUtil::defaultAttributes() return attributes; } -// LINUX -// ----- -#if defined(BSLS_PLATFORM_OS_LINUX) -const bool ThreadUtil::k_SUPPORT_THREAD_NAME = true; -#else -const bool ThreadUtil::k_SUPPORT_THREAD_NAME = false; -#endif - void ThreadUtil::setCurrentThreadName(const bsl::string& value) { bslmt::ThreadUtil::setThreadName(value); diff --git a/src/groups/bmq/bmqsys/bmqsys_threadutil.h b/src/groups/bmq/bmqsys/bmqsys_threadutil.h index 1faeb3f4be..3c507cd789 100644 --- a/src/groups/bmq/bmqsys/bmqsys_threadutil.h +++ b/src/groups/bmq/bmqsys/bmqsys_threadutil.h @@ -47,12 +47,6 @@ namespace bmqsys { /// Utility namespace for thread management struct ThreadUtil { - // CONSTANTS - - /// Boolean constant indicating whether the current platform supports - /// naming thread. - static const bool k_SUPPORT_THREAD_NAME; - // CLASS METHODS /// Return `bslmt::ThreadAttributes` object pre-initialized with default