From b77e796c90acf2cc7762b64a721017ea1f53cf2f Mon Sep 17 00:00:00 2001 From: Matt Millett Date: Fri, 21 Jun 2024 14:08:05 -0400 Subject: [PATCH 1/3] Fix incorrect mutex implementation selector when futexes are desired --- groups/ntc/ntccfg/ntccfg_mutex.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/groups/ntc/ntccfg/ntccfg_mutex.h b/groups/ntc/ntccfg/ntccfg_mutex.h index 86d3b7c9..eba67f66 100644 --- a/groups/ntc/ntccfg/ntccfg_mutex.h +++ b/groups/ntc/ntccfg/ntccfg_mutex.h @@ -345,7 +345,7 @@ typedef bslmt::LockGuard LockGuard; /// @ingroup module_ntccfg typedef bslmt::UnLockGuard UnLockGuard; -#elif NTCCFG_MUTEX_IMPL == NTCCFG_MUTEX_IMPL_BSLMT_FUTEX +#elif NTCCFG_MUTEX_IMPL == NTCCFG_MUTEX_IMPL_FUTEX /// @internal @brief /// Provide a synchronization primitive for mutually-exclusive access. From 23ad442a2f607d19b318d62b0a3df3daf341fe6c Mon Sep 17 00:00:00 2001 From: Matt Millett Date: Fri, 21 Jun 2024 14:11:39 -0400 Subject: [PATCH 2/3] Rename futex implementation from ntccfg::Mutex to Futex and typedef to Mutex --- groups/ntc/ntccfg/ntccfg_mutex.cpp | 4 ++-- groups/ntc/ntccfg/ntccfg_mutex.h | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/groups/ntc/ntccfg/ntccfg_mutex.cpp b/groups/ntc/ntccfg/ntccfg_mutex.cpp index f63113e3..50abd22b 100644 --- a/groups/ntc/ntccfg/ntccfg_mutex.cpp +++ b/groups/ntc/ntccfg/ntccfg_mutex.cpp @@ -35,7 +35,7 @@ namespace ntccfg { #if NTCCFG_FUTEX_ENABLED // Some versions of GCC issue a spurious warning that the 'current' paramter -// is set but not used when 'Mutex::compareAndSwap' is called. +// is set but not used when 'Futex::compareAndSwap' is called. #if defined(BSLS_PLATFORM_CMP_GNU) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-but-set-parameter" @@ -51,7 +51,7 @@ __attribute__((noinline)) void Futex::wake() syscall(SYS_futex, (int*)(&d_value), FUTEX_WAKE, 1, 0, 0, 0); } -__attribute__((noinline)) void Mutex::lockContention(int c) +__attribute__((noinline)) void Futex::lockContention(int c) { do { if (c == 2 || compareAndSwap(&d_value, 1, 2) != 0) { diff --git a/groups/ntc/ntccfg/ntccfg_mutex.h b/groups/ntc/ntccfg/ntccfg_mutex.h index eba67f66..370ac552 100644 --- a/groups/ntc/ntccfg/ntccfg_mutex.h +++ b/groups/ntc/ntccfg/ntccfg_mutex.h @@ -42,7 +42,7 @@ namespace ntccfg { #if NTCCFG_FUTEX_ENABLED // Some versions of GCC issue a spurious warning that the 'current' paramter -// is set but not used when 'Mutex::compareAndSwap' is called. +// is set but not used when 'Futex::compareAndSwap' is called. #if defined(BSLS_PLATFORM_CMP_GNU) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-but-set-parameter" @@ -99,7 +99,7 @@ class __attribute__((__aligned__(sizeof(int)))) Futex }; NTCCFG_INLINE -int Mutex::compareAndSwap(int* current, int expected, int desired) +int Futex::compareAndSwap(int* current, int expected, int desired) { int* ep = &expected; int* dp = &desired; @@ -115,18 +115,18 @@ int Mutex::compareAndSwap(int* current, int expected, int desired) } NTCCFG_INLINE -Mutex::Mutex() +Futex::Futex() { __atomic_store_n(&d_value, 0, __ATOMIC_RELAXED); } NTCCFG_INLINE -Mutex::~Mutex() +Futex::~Futex() { } NTCCFG_INLINE -void Mutex::lock() +void Futex::lock() { int previous = compareAndSwap(&d_value, 0, 1); if (previous != 0) { @@ -135,7 +135,7 @@ void Mutex::lock() } NTCCFG_INLINE -void Mutex::unlock() +void Futex::unlock() { int previous = __atomic_fetch_sub(&d_value, 1, __ATOMIC_SEQ_CST); if (previous != 1) { From 96982ba8929b26c6c73173affc348e1a71e40144 Mon Sep 17 00:00:00 2001 From: Matt Millett Date: Fri, 21 Jun 2024 14:51:45 -0400 Subject: [PATCH 3/3] Use lock guards suitable for configurable mutex implementations --- groups/ntc/ntcp/ntcp_interface.cpp | 26 ++++++++++---------- groups/ntc/ntcp/ntcp_interface.h | 38 +++++++++++++++--------------- groups/ntc/ntcr/ntcr_interface.cpp | 26 ++++++++++---------- groups/ntc/ntcr/ntcr_interface.h | 38 +++++++++++++++--------------- 4 files changed, 64 insertions(+), 64 deletions(-) diff --git a/groups/ntc/ntcp/ntcp_interface.cpp b/groups/ntc/ntcp/ntcp_interface.cpp index 38472688..b1229802 100644 --- a/groups/ntc/ntcp/ntcp_interface.cpp +++ b/groups/ntc/ntcp/ntcp_interface.cpp @@ -335,7 +335,7 @@ ntsa::Error Interface::addThread() bsl::shared_ptr Interface::acquireProactorUsedByThreadHandle( const ntca::LoadBalancingOptions& options) { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); bsl::shared_ptr result; @@ -366,7 +366,7 @@ bsl::shared_ptr Interface::acquireProactorUsedByThreadHandle( bsl::shared_ptr Interface::acquireProactorUsedByThreadIndex( const ntca::LoadBalancingOptions& options) { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); bsl::shared_ptr result; @@ -401,7 +401,7 @@ bsl::shared_ptr Interface::acquireProactorUsedByThreadIndex( bsl::shared_ptr Interface::acquireProactorWithLeastLoad( const ntca::LoadBalancingOptions& options) { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); NTCI_LOG_CONTEXT(); @@ -563,7 +563,7 @@ ntsa::Error Interface::start() bsl::shared_ptr resolver; { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); if (!d_resolver_sp) { BSLS_ASSERT_OPT(!d_config.resolverEnabled().isNull()); @@ -618,7 +618,7 @@ void Interface::shutdown() bsl::shared_ptr resolver; ProactorVector proactorVector(d_allocator_p); { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); resolver = d_resolver_sp; proactorVector = d_proactorVector; @@ -645,7 +645,7 @@ void Interface::linger() ThreadVector threadVector(d_allocator_p); ProactorVector proactorVector(d_allocator_p); { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); resolver = d_resolver_sp; threadVector = d_threadVector; @@ -681,7 +681,7 @@ void Interface::linger() proactorVector.clear(); { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); d_threadVector.clear(); d_threadMap.clear(); @@ -698,7 +698,7 @@ ntsa::Error Interface::closeAll() ProactorVector proactorVector(d_allocator_p); { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); proactorVector = d_proactorVector; } @@ -1489,7 +1489,7 @@ void Interface::releaseHandleReservation() bool Interface::expand() { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); NTCI_LOG_CONTEXT(); @@ -1510,13 +1510,13 @@ bool Interface::expand() bsl::size_t Interface::numProactors() const { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); return d_proactorVector.size(); } bsl::size_t Interface::numThreads() const { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); return d_threadVector.size(); } @@ -1544,7 +1544,7 @@ bool Interface::lookupByThreadHandle( bsl::shared_ptr* result, bslmt::ThreadUtil::Handle threadHandle) const { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); result->reset(); @@ -1575,7 +1575,7 @@ bool Interface::lookupByThreadHandle( bool Interface::lookupByThreadIndex(bsl::shared_ptr* result, bsl::size_t threadIndex) const { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); result->reset(); diff --git a/groups/ntc/ntcp/ntcp_interface.h b/groups/ntc/ntcp/ntcp_interface.h index d2537c7e..be3889f1 100644 --- a/groups/ntc/ntcp/ntcp_interface.h +++ b/groups/ntc/ntcp/ntcp_interface.h @@ -76,28 +76,28 @@ class Interface : public ntci::Interface, /// Define a type alias for a vector of proactors. typedef bsl::vector > ProactorVector; - ntccfg::Object d_object; - - mutable ntccfg::Mutex d_mutex; - - bsl::shared_ptr d_user_sp; - bsl::shared_ptr d_dataPool_sp; - bsl::shared_ptr d_resolver_sp; - - bsl::shared_ptr d_connectionLimiter_sp; - bsl::shared_ptr d_socketMetrics_sp; - + // Define a type alias for a mutex. + typedef ntccfg::Mutex Mutex; + + /// Define a type alias for a mutex lock guard. + typedef ntccfg::LockGuard LockGuard; + + ntccfg::Object d_object; + mutable Mutex d_mutex; + bsl::shared_ptr d_user_sp; + bsl::shared_ptr d_dataPool_sp; + bsl::shared_ptr d_resolver_sp; + bsl::shared_ptr d_connectionLimiter_sp; + bsl::shared_ptr d_socketMetrics_sp; bsl::shared_ptr d_proactorFactory_sp; bsl::shared_ptr d_proactorMetrics_sp; ProactorVector d_proactorVector; - - ThreadVector d_threadVector; - ThreadMap d_threadMap; - bslmt::Semaphore d_threadSemaphore; - bsl::size_t d_threadWatermark; - - ntca::InterfaceConfig d_config; - bslma::Allocator* d_allocator_p; + ThreadVector d_threadVector; + ThreadMap d_threadMap; + bslmt::Semaphore d_threadSemaphore; + bsl::size_t d_threadWatermark; + ntca::InterfaceConfig d_config; + bslma::Allocator* d_allocator_p; private: Interface(const Interface&) BSLS_KEYWORD_DELETED; diff --git a/groups/ntc/ntcr/ntcr_interface.cpp b/groups/ntc/ntcr/ntcr_interface.cpp index 01b9ff10..4e3700df 100644 --- a/groups/ntc/ntcr/ntcr_interface.cpp +++ b/groups/ntc/ntcr/ntcr_interface.cpp @@ -344,7 +344,7 @@ ntsa::Error Interface::addThread() bsl::shared_ptr Interface::acquireReactorUsedByThreadHandle( const ntca::LoadBalancingOptions& options) { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); bsl::shared_ptr result; @@ -375,7 +375,7 @@ bsl::shared_ptr Interface::acquireReactorUsedByThreadHandle( bsl::shared_ptr Interface::acquireReactorUsedByThreadIndex( const ntca::LoadBalancingOptions& options) { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); bsl::shared_ptr result; @@ -410,7 +410,7 @@ bsl::shared_ptr Interface::acquireReactorUsedByThreadIndex( bsl::shared_ptr Interface::acquireReactorWithLeastLoad( const ntca::LoadBalancingOptions& options) { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); NTCI_LOG_CONTEXT(); @@ -572,7 +572,7 @@ ntsa::Error Interface::start() bsl::shared_ptr resolver; { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); if (!d_resolver_sp) { BSLS_ASSERT_OPT(!d_config.resolverEnabled().isNull()); @@ -627,7 +627,7 @@ void Interface::shutdown() bsl::shared_ptr resolver; ReactorVector reactorVector(d_allocator_p); { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); resolver = d_resolver_sp; reactorVector = d_reactorVector; @@ -654,7 +654,7 @@ void Interface::linger() ThreadVector threadVector(d_allocator_p); ReactorVector reactorVector(d_allocator_p); { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); resolver = d_resolver_sp; threadVector = d_threadVector; @@ -690,7 +690,7 @@ void Interface::linger() reactorVector.clear(); { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); d_threadVector.clear(); d_threadMap.clear(); @@ -707,7 +707,7 @@ ntsa::Error Interface::closeAll() ReactorVector reactorVector(d_allocator_p); { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); reactorVector = d_reactorVector; } @@ -1499,7 +1499,7 @@ void Interface::releaseHandleReservation() bool Interface::expand() { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); NTCI_LOG_CONTEXT(); @@ -1520,13 +1520,13 @@ bool Interface::expand() bsl::size_t Interface::numReactors() const { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); return d_reactorVector.size(); } bsl::size_t Interface::numThreads() const { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); return d_threadVector.size(); } @@ -1554,7 +1554,7 @@ bool Interface::lookupByThreadHandle( bsl::shared_ptr* result, bslmt::ThreadUtil::Handle threadHandle) const { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); result->reset(); @@ -1585,7 +1585,7 @@ bool Interface::lookupByThreadHandle( bool Interface::lookupByThreadIndex(bsl::shared_ptr* result, bsl::size_t threadIndex) const { - bslmt::LockGuard lock(&d_mutex); + LockGuard lock(&d_mutex); result->reset(); diff --git a/groups/ntc/ntcr/ntcr_interface.h b/groups/ntc/ntcr/ntcr_interface.h index 90581e34..6ad59077 100644 --- a/groups/ntc/ntcr/ntcr_interface.h +++ b/groups/ntc/ntcr/ntcr_interface.h @@ -77,28 +77,28 @@ class Interface : public ntci::Interface, /// Define a type alias for a vector of reactors. typedef bsl::vector > ReactorVector; - ntccfg::Object d_object; - - mutable ntccfg::Mutex d_mutex; - - bsl::shared_ptr d_user_sp; - bsl::shared_ptr d_dataPool_sp; - bsl::shared_ptr d_resolver_sp; - - bsl::shared_ptr d_connectionLimiter_sp; - bsl::shared_ptr d_socketMetrics_sp; - + /// Define a type alias for a mutex. + typedef ntccfg::Mutex Mutex; + + /// Define a type alias for a mutex lock guard. + typedef ntccfg::LockGuard LockGuard; + + ntccfg::Object d_object; + mutable Mutex d_mutex; + bsl::shared_ptr d_user_sp; + bsl::shared_ptr d_dataPool_sp; + bsl::shared_ptr d_resolver_sp; + bsl::shared_ptr d_connectionLimiter_sp; + bsl::shared_ptr d_socketMetrics_sp; bsl::shared_ptr d_reactorFactory_sp; bsl::shared_ptr d_reactorMetrics_sp; ReactorVector d_reactorVector; - - ThreadVector d_threadVector; - ThreadMap d_threadMap; - bslmt::Semaphore d_threadSemaphore; - bsl::size_t d_threadWatermark; - - ntca::InterfaceConfig d_config; - bslma::Allocator* d_allocator_p; + ThreadVector d_threadVector; + ThreadMap d_threadMap; + bslmt::Semaphore d_threadSemaphore; + bsl::size_t d_threadWatermark; + ntca::InterfaceConfig d_config; + bslma::Allocator* d_allocator_p; private: Interface(const Interface&) BSLS_KEYWORD_DELETED;