Fix[mqbblp::Domain]: unsafe d_queues access without mutex - #1670
Conversation
04f81fa to
0b1e616
Compare
| true, // isReconfigure | ||
| false); // wait | ||
| d_dispatcher_p->execute(reconfigureQueueFn, cluster()); | ||
| } |
There was a problem hiding this comment.
it->second.get() was a raw ptr to a queue. What if a queue is destructed in between? To handle this, I changed the snapshot to take weak_ptr.
| false); // wait | ||
| d_dispatcher_p->execute(reconfigureQueueFn, cluster()); | ||
| } | ||
| d_dispatcher_p->execute(QueueReconfigureFunctor(queues_sp), cluster()); |
There was a problem hiding this comment.
If domain has thousands of queues, we enqueued thousands of events here.
At least on this level I wanted to enqueue only one event and made functor for it, that owns queues snapshot
0b1e616 to
901cf4a
Compare
| } | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
Each one of these queue->configure enqueues an event to its dispatcher thread, it is the existing behavior and I decided not to change it in this PR
901cf4a to
9aeb03c
Compare
| BMQTST_ASSERT_EQ(static_cast<int>(queues.size()), k_NUM_QUEUES); | ||
| } | ||
|
|
||
| static void test3_reconfigureSkipsUnregisteredQueue() |
There was a problem hiding this comment.
This test fails if we try to bind raw ptr to a queue, like it was done before
ab72d6e to
3036a96
Compare
d_queues access without mutex
3036a96 to
5762500
Compare
Add `mqbblp_domain.t.cpp` test driver: - `test1_breathingTest` — construct / configure / reconfigure / teardown. - `test2_concurrentConfigureAndRegisterQueue` — one thread reconfigures continuously while the main thread registers thousands of distinct queues, exercising the exact overlap. - `test3_reconfigureSkipsUnregisteredQueue` — deterministically (via the mock dispatcher's enqueueOnly mode) unregisters and destroys a queue after the reconfigure is posted but before it runs, asserting the dispatched work skips the dead queue (weak_ptr gone) and reconfigures the survivor exactly once — proving no raw/owning reference outlives the queue. - `test4_concurrentProcessCommandAndRegisterQueue` — one thread issues `INFO` commands continuously while the main thread register thousands of distinct queues, exercising the `processCommand` overlap. Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
Fixes the following bugs: **Bug 1**: `mqbblp::Domain::configure` iterated the `d_queues` map on the admin thread without holding `d_mutex`, while `registerQueue`/`unregisterQueue` mutate the same map (insert/erase) under `d_mutex` on the cluster dispatcher thread. A `DOMAINS RECONFIGURE` overlapping a queue open/close is a data race - a rehash on insert could invalidate the concurrent iterator (crash / garbage read). Fixed by making a snapshot under the mutex. **Bug 2**: The reconfigure path also bound a raw `mqbi::Queue*` (`it->second.get()`) into an async dispatcher functor, opening a use-after-free window between enqueue and execution if the queue was unregistered in the meantime. Fixed by holding a `weak_ptr` in queues snapshot (no raw pointer stored on this level). Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
`Domain::processCommand` makes a copy of `d_queues` without holding a mutex from ADMIN thread. `d_queues` might be changed or rehashed from the cluster dispatcher thread at the same time. Fixed by making a snapshot under the mutex. Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
…utex `Domain::registerQueue` logs `d_queues.size()` without holding a mutex. Fixed by storing size for logging under the mutex. Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
5762500 to
240fa5e
Compare
| switch (_testCase) { | ||
| case 4: test4_concurrentProcessCommandAndRegisterQueue(); break; | ||
| case 3: test3_reconfigureSkipsUnregisteredQueue(); break; | ||
| case 2: test2_concurrentConfigureAndRegisterQueue(); break; |
There was a problem hiding this comment.
Fails on main
[18:41:56] CASE 2: FAILURE (rc 66)
TEST /blazingmq/src/groups/mqb/mqbblp/mqbblp_domain.t.cpp CASE 2
==================
WARNING: ThreadSanitizer: data race (pid=2542)
Read of size 8 at 0x726000000688 by thread T3:
#0 BloombergLP::bslalg::HashTableAnchor::listRootAddress() const /opt/bb/include/bslalg_hashtableanchor.h:649:12 (mqbblp_domain.t+0x87db6d) (BuildId: 97db8b4d024fa5340c1abf5[310](https://github.com/bloomberg/blazingmq/actions/runs/30564850980/job/90957989948#step:5:311)c63101bf59d70b)
#1 BloombergLP::bslstl::HashTable<BloombergLP::bslstl::UnorderedMapKeyConfiguration<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>> const, bsl::pair<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>> const, bsl::shared_ptr<BloombergLP::mqbi::Queue>>>, bsl::hash<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>>>, bsl::equal_to<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>>>, bsl::allocator<bsl::pair<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>> const, bsl::shared_ptr<BloombergLP::mqbi::Queue>>>>::elementListRoot() const /opt/bb/include/bslstl_hashtable.h:4978:21 (mqbblp_domain.t+0x886559) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#2 bsl::unordered_map<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>>, bsl::shared_ptr<BloombergLP::mqbi::Queue>, bsl::hash<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>>>, bsl::equal_to<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>>>, bsl::allocator<bsl::pair<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>> const, bsl::shared_ptr<BloombergLP::mqbi::Queue>>>>::begin() /opt/bb/include/bslstl_unorderedmap.h:3259:28 (mqbblp_domain.t+0x8707f5) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#3 BloombergLP::mqbblp::Domain::configure(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, BloombergLP::mqbconfm::Domain const&) /blazingmq/src/groups/mqb/mqbblp/mqbblp_domain.cpp:376:42 (mqbblp_domain.t+0x869167) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#4 (anonymous namespace)::reconfigureThread(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*) /blazingmq/src/groups/mqb/mqbblp/mqbblp_domain.t.cpp:335:17 (mqbblp_domain.t+0x856be6) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#5 void BloombergLP::bdlf::Bind_Invoker<void, 4>::invoke<void (* const)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*> const, BloombergLP::bdlf::Bind_ArgTuple0>(void (* const*)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*> const*, BloombergLP::bdlf::Bind_ArgTuple0&) const /opt/bb/include/bdlf_bind.h:8685:9 (mqbblp_domain.t+0x861d53) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#6 void BloombergLP::bdlf::Bind_ImplExplicit<BloombergLP::bslmf::Nil, void (*)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*>>::invokeImpl<BloombergLP::bdlf::Bind_ArgTuple0>(BloombergLP::bdlf::Bind_ArgTuple0&, BloombergLP::bslmf::Tag<0u>) const /opt/bb/include/bdlf_bind.h:5950:24 (mqbblp_domain.t+0x861bcc) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#7 void BloombergLP::bdlf::Bind_ImplExplicit<BloombergLP::bslmf::Nil, void (*)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*>>::invoke<BloombergLP::bdlf::Bind_ArgTuple0>(BloombergLP::bdlf::Bind_ArgTuple0&) const /opt/bb/include/bdlf_bind.h:6014:16 (mqbblp_domain.t+0x861b49) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#8 BloombergLP::bdlf::Bind_ImplExplicit<BloombergLP::bslmf::Nil, void (*)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*>>::operator()() const /opt/bb/include/bdlf_bind.h:6023:16 (mqbblp_domain.t+0x86194e) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#9 BloombergLP::bslmt::EntryPointFunctorAdapter<BloombergLP::bdlf::Bind<BloombergLP::bslmf::Nil, void (*)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*>>>::invokerFunction(void*) /opt/bb/include/bslmt_entrypointfunctoradapter.h:324:5 (mqbblp_domain.t+0x8615e4) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#10 bslmt_EntryPointFunctorAdapter_invoker /blazingmq/deps/srcs/bde/groups/bsl/bslmt/bslmt_entrypointfunctoradapter.cpp:15:5 (mqbblp_domain.t+0x14075a3) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
|
|
||
| switch (_testCase) { | ||
| case 4: test4_concurrentProcessCommandAndRegisterQueue(); break; | ||
| case 3: test3_reconfigureSkipsUnregisteredQueue(); break; |
There was a problem hiding this comment.
Fails on main
[18:41:55] CASE 3: FAILURE (rc 66)
TEST /blazingmq/src/groups/mqb/mqbblp/mqbblp_domain.t.cpp CASE 3
ThreadSanitizer:DEADLYSIGNAL
==2549==ERROR: ThreadSanitizer: SEGV on unknown address 0x7fdd2be39ad0 (pc 0x7fdd2be39ad0 bp 0x7ffe5c997430 sp 0x7ffe5c9973a8 T2549)
==2549==The signal is caused by a READ memory access.
==2549==Hint: PC is at a non-executable region. Maybe a wild jump?
#0 vtable for __cxxabiv1::__class_type_info <null> (libc++abi.so.1+0x91ad0) (BuildId: 2ec6bcab459ee90b682da1d4df59788d21f1935b)
#1 int BloombergLP::bdlf::Bind_Invoker<int, 4>::invoke<BloombergLP::bdlf::MemFn<int (BloombergLP::mqbi::Queue::*)(std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool)> const, BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbi::Queue*, std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool> const, BloombergLP::bdlf::Bind_ArgTuple0>(BloombergLP::bdlf::MemFn<int (BloombergLP::mqbi::Queue::*)(std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool)> const*, BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbi::Queue*, std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool> const*, BloombergLP::bdlf::Bind_ArgTuple0&) const /opt/bb/include/bdlf_bind.h:8672:16 (mqbblp_domain.t+0x8883b3) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#2 int BloombergLP::bdlf::Bind_ImplExplicit<BloombergLP::bslmf::Nil, int (BloombergLP::mqbi::Queue::*)(std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbi::Queue*, std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool>>::invokeImpl<BloombergLP::bdlf::Bind_ArgTuple0>(BloombergLP::bdlf::Bind_ArgTuple0&, BloombergLP::bslmf::Tag<0u>) const /opt/bb/include/bdlf_bind.h:5950:24 (mqbblp_domain.t+0x88823c) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#3 int BloombergLP::bdlf::Bind_ImplExplicit<BloombergLP::bslmf::Nil, int (BloombergLP::mqbi::Queue::*)(std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbi::Queue*, std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool>>::invoke<BloombergLP::bdlf::Bind_ArgTuple0>(BloombergLP::bdlf::Bind_ArgTuple0&) const /opt/bb/include/bdlf_bind.h:6014:16 (mqbblp_domain.t+0x8881b9) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#4 BloombergLP::bdlf::Bind_ImplExplicit<BloombergLP::bslmf::Nil, int (BloombergLP::mqbi::Queue::*)(std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbi::Queue*, std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool>>::operator()() const /opt/bb/include/bdlf_bind.h:6023:16 (mqbblp_domain.t+0x88812e) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#5 BloombergLP::bslstl::Function_InvokerUtil_Dispatch<4, int (), BloombergLP::bdlf::Bind<BloombergLP::bslmf::Nil, int (BloombergLP::mqbi::Queue::*)(std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbi::Queue*, std::__1::basic_ostream<char, std::__1::char_traits<char>>*, bool, bool>>>::invoke(BloombergLP::bslstl::Function_Rep const*) /opt/bb/include/bslstl_function_invokerutil.h:951:12 (mqbblp_domain.t+0x8880b2) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#6 BloombergLP::bslstl::Function_Variadic<int ()>::operator()() const /opt/bb/include/bslstl_function.h:1545:12 (mqbblp_domain.t+0x88a340) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#7 BloombergLP::bslstl::Function_InvokerUtil_Dispatch<5, void (), bsl::function<int ()>>::invoke(BloombergLP::bslstl::Function_Rep const*) /opt/bb/include/bslstl_function_invokerutil.h:967:12 (mqbblp_domain.t+0x88a1f2) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#8 BloombergLP::bslstl::Function_Variadic<void ()>::operator()() const /opt/bb/include/bslstl_function.h:1545:12 (mqbblp_domain.t+0x90de30) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#9 BloombergLP::mqbmock::Dispatcher::processQueue() /blazingmq/src/groups/mqb/mqbmock/mqbmock_dispatcher.cpp:154:9 (mqbblp_domain.t+0xd59c10) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#10 test3_reconfigureSkipsUnregisteredQueue() /blazingmq/src/groups/mqb/mqbblp/mqbblp_domain.t.cpp:492:25 (mqbblp_domain.t+0x851182) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#11 main /blazingmq/src/groups/mqb/mqbblp/mqbblp_domain.t.cpp:619:17 (mqbblp_domain.t+0x850082) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#12 <null> <null> (libc.so.6+0x2a1c9) (BuildId: 3[288](https://github.com/bloomberg/blazingmq/actions/runs/30564850980/job/90957989948#step:5:289)20b908de8ea1ef79afa8995e302e819163d7)
#13 __libc_start_main <null> (libc.so.6+0x2a28a) (BuildId: 328820b908de8ea1ef79afa8995e302e819163d7)
#14 _start <null> (mqbblp_domain.t+0x7a0694) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
| mqbcfg::BrokerConfig::set(brokerConfig); | ||
|
|
||
| switch (_testCase) { | ||
| case 4: test4_concurrentProcessCommandAndRegisterQueue(); break; |
There was a problem hiding this comment.
Fails on main
[18:41:56] CASE 4: FAILURE (rc 66)
TEST /blazingmq/src/groups/mqb/mqbblp/mqbblp_domain.t.cpp CASE 4
==================
WARNING: ThreadSanitizer: data race (pid=2556)
Read of size 8 at 0x726000000688 by thread T3:
#0 BloombergLP::bslalg::HashTableAnchor::listRootAddress() const /opt/bb/include/bslalg_hashtableanchor.h:649:12 (mqbblp_domain.t+0x87db6d) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#1 BloombergLP::bslstl::HashTable<BloombergLP::bslstl::UnorderedMapKeyConfiguration<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>> const, bsl::pair<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>> const, bsl::shared_ptr<BloombergLP::mqbi::Queue>>>, bsl::hash<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>>>, bsl::equal_to<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>>>, bsl::allocator<bsl::pair<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>> const, bsl::shared_ptr<BloombergLP::mqbi::Queue>>>>::elementListRoot() const /opt/bb/include/bslstl_hashtable.h:4978:21 (mqbblp_domain.t+0x886559) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#2 bsl::unordered_map<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>>, bsl::shared_ptr<BloombergLP::mqbi::Queue>, bsl::hash<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>>>, bsl::equal_to<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>>>, bsl::allocator<bsl::pair<bsl::basic_string<char, std::__1::char_traits<char>, bsl::allocator<char>> const, bsl::shared_ptr<BloombergLP::mqbi::Queue>>>>::cbegin() const /opt/bb/include/bslstl_unorderedmap.h:3768:34 (mqbblp_domain.t+0x8731a5) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#3 BloombergLP::mqbblp::Domain::processCommand(BloombergLP::mqbcmd::DomainResult*, BloombergLP::mqbcmd::DomainCommand const&) /blazingmq/src/groups/mqb/mqbblp/mqbblp_domain.cpp:666:43 (mqbblp_domain.t+0x86ca5c) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#4 (anonymous namespace)::processInfoThread(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*) /blazingmq/src/groups/mqb/mqbblp/mqbblp_domain.t.cpp:531:17 (mqbblp_domain.t+0x[853](https://github.com/bloomberg/blazingmq/actions/runs/30564850980/job/90957989948#step:5:854)1dd) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#5 void BloombergLP::bdlf::Bind_Invoker<void, 4>::invoke<void (* const)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*> const, BloombergLP::bdlf::Bind_ArgTuple0>(void (* const*)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*> const*, BloombergLP::bdlf::Bind_ArgTuple0&) const /opt/bb/include/bdlf_bind.h:8685:9 (mqbblp_domain.t+0x861d53) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#6 void BloombergLP::bdlf::Bind_ImplExplicit<BloombergLP::bslmf::Nil, void (*)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*>>::invokeImpl<BloombergLP::bdlf::Bind_ArgTuple0>(BloombergLP::bdlf::Bind_ArgTuple0&, BloombergLP::bslmf::Tag<0u>) const /opt/bb/include/bdlf_bind.h:5950:24 (mqbblp_domain.t+0x861bcc) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#7 void BloombergLP::bdlf::Bind_ImplExplicit<BloombergLP::bslmf::Nil, void (*)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*>>::invoke<BloombergLP::bdlf::Bind_ArgTuple0>(BloombergLP::bdlf::Bind_ArgTuple0&) const /opt/bb/include/bdlf_bind.h:6014:16 (mqbblp_domain.t+0x861b49) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#8 BloombergLP::bdlf::Bind_ImplExplicit<BloombergLP::bslmf::Nil, void (*)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*>>::operator()() const /opt/bb/include/bdlf_bind.h:6023:16 (mqbblp_domain.t+0x86194e) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#9 BloombergLP::bslmt::EntryPointFunctorAdapter<BloombergLP::bdlf::Bind<BloombergLP::bslmf::Nil, void (*)(BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*), BloombergLP::bdlf::Bind_BoundTuple4<BloombergLP::mqbblp::Domain*, BloombergLP::bslmt::Barrier*, BloombergLP::bsls::AtomicBool*, BloombergLP::bslma::Allocator*>>>::invokerFunction(void*) /opt/bb/include/bslmt_entrypointfunctoradapter.h:324:5 (mqbblp_domain.t+0x8615e4) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
#10 bslmt_EntryPointFunctorAdapter_invoker /blazingmq/deps/srcs/bde/groups/bsl/bslmt/bslmt_entrypointfunctoradapter.cpp:15:5 (mqbblp_domain.t+0x14075a3) (BuildId: 97db8b4d024fa5340c1abf5310c63101bf59d70b)
|
UTs run on main for this PR: #1671 |
Problem
Bug 1:
mqbblp::Domain::configureiterated thed_queuesmap on the admin thread without holdingd_mutex, whileregisterQueue/unregisterQueuemutate the same map (insert/erase) underd_mutexon the cluster dispatcher thread. ADOMAINS RECONFIGUREoverlapping a queue open/close is a data race - a rehash on insert could invalidate the concurrent iterator (crash / garbage read).Bug 2:
The reconfigure path also bound a raw
mqbi::Queue*(it->second.get()) into an async dispatcher functor, opening a use-after-free window between enqueue and execution if the queue was unregistered in the meantime.Bug 3:
Domain::processCommandmakes a copy ofd_queueswithout holding a mutex from ADMIN thread.d_queuesmight be changed or rehashed from the cluster dispatcher thread at the same time.Bug 4:
Domain::registerQueuelogsd_queues.size()without holding a mutex.Fix
In
Domain::configure, snapshot the queues underd_mutexinto ashared_ptr<vector<weak_ptr<mqbi::Queue>>>, then dispatch a single functor (QueueReconfigureFunctor) that owns the snapshot. On the cluster dispatcher thread itlock()s each weak pointer and reconfigures only the queues still alive — queues unregistered since the snapshot are safely skipped.This closes both the iteration race (map is now only touched under the mutex) and the lifetime hazard (no raw pointers bound;
weak_ptr::lock()gates every access).Tests
Add
mqbblp_domain.t.cpptest driver:test1_breathingTest— construct / configure / reconfigure / teardown.test2_concurrentConfigureAndRegisterQueue— one thread reconfigures continuously while the main thread registers thousands of distinct queues, exercising the exact overlap.test3_reconfigureSkipsUnregisteredQueue— deterministically (via the mock dispatcher's enqueueOnly mode) unregisters and destroys a queue after the reconfigure is posted but before it runs, asserting the dispatched work skips the dead queue (weak_ptr gone) and reconfigures the survivor exactly once — proving no raw/owning reference outlives the queue.test4_concurrentProcessCommandAndRegisterQueue— one thread issuesINFOcommands continuously while the main thread register thousands of distinct queues, exercising theprocessCommandoverlap.