Skip to content

Commit 901cf4a

Browse files
committed
Fix[mqbblp::Domain]: reconfigure iterates queues without mutex
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
1 parent 9ee0a6e commit 901cf4a

3 files changed

Lines changed: 486 additions & 20 deletions

File tree

src/groups/mqb/mqbblp/mqbblp_domain.cpp

Lines changed: 79 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,60 @@ int normalizeConfig(mqbconfm::Domain* defn,
211211
return updatedValues;
212212
}
213213

214+
/// @brief Function object that reconfigures a snapshot of queues.
215+
///
216+
/// Owns a shared snapshot of weak queue pointers, taken under the domain's
217+
/// mutex. When invoked (on the cluster dispatcher thread), it locks each
218+
/// weak pointer and reconfigures the queues that are still alive; queues
219+
/// that have been unregistered since the snapshot was taken are skipped.
220+
class QueueReconfigureFunctor {
221+
public:
222+
// TYPES
223+
224+
/// Snapshot of queues held by weak pointer.
225+
typedef bsl::vector<bsl::weak_ptr<mqbi::Queue> > QueueWeakPtrVector;
226+
227+
private:
228+
// DATA
229+
230+
/// Shared snapshot of the queues to reconfigure.
231+
bsl::shared_ptr<QueueWeakPtrVector> d_queues_sp;
232+
233+
public:
234+
// CREATORS
235+
236+
/// @brief Create a functor owning the specified `queues_sp` snapshot.
237+
///
238+
/// @param queues_sp The queues to reconfigure, held by weak pointer.
239+
explicit QueueReconfigureFunctor(
240+
const bsl::shared_ptr<QueueWeakPtrVector>& queues_sp)
241+
: d_queues_sp(queues_sp)
242+
{
243+
// PRECONDITIONS
244+
BSLS_ASSERT(queues_sp);
245+
}
246+
247+
// ACCESSORS
248+
249+
/// @brief Reconfigure every queue in the snapshot that is still alive.
250+
void operator()() const
251+
{
252+
// Thread: CLUSTER DISPATCHER
253+
254+
for (QueueWeakPtrVector::const_iterator it = d_queues_sp->begin();
255+
it != d_queues_sp->end();
256+
++it) {
257+
bsl::shared_ptr<mqbi::Queue> queue = it->lock();
258+
if (queue) {
259+
queue->configure(
260+
static_cast<bsl::ostream*>(0), // errorDescription
261+
true, // isReconfigure
262+
false); // wait
263+
}
264+
}
265+
}
266+
};
267+
214268
} // close unnamed namespace
215269

216270
// ------------
@@ -224,7 +278,7 @@ void Domain::onOpenQueueResponse(
224278
const mqbi::OpenQueueConfirmationCookieSp& confirmationCookie,
225279
const mqbi::Domain::OpenQueueCallback& callback)
226280
{
227-
// executed by *ANY* thread
281+
// Thread: ANY
228282

229283
--d_pendingRequests;
230284
if (status.category() == bmqp_ctrlmsg::StatusCategory::E_SUCCESS) {
@@ -287,6 +341,8 @@ Domain::~Domain()
287341
int Domain::configure(bsl::ostream& errorDescription,
288342
const mqbconfm::Domain& newConfig)
289343
{
344+
// Thread: ADMIN
345+
290346
enum RcEnum {
291347
// Value for the various RC error categories
292348
rc_SUCCESS = 0,
@@ -358,6 +414,22 @@ int Domain::configure(bsl::ostream& errorDescription,
358414
// any needed update-advisories to the CSL.
359415
d_cluster_sp->onDomainReconfigured(*this, *oldConfig, finalConfig);
360416

417+
// Make a snapshot of weak queue pointers for reconfigure.
418+
bsl::shared_ptr<QueueReconfigureFunctor::QueueWeakPtrVector>
419+
queues_sp = bsl::allocate_shared<
420+
QueueReconfigureFunctor::QueueWeakPtrVector>(d_allocator_p);
421+
{
422+
bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex); // LOCK
423+
queues_sp->reserve(d_queues.size());
424+
for (QueueMapCIter it = d_queues.begin(); it != d_queues.end();
425+
++it) {
426+
queues_sp->push_back(it->second);
427+
}
428+
}
429+
430+
BALL_LOG_INFO << "Reconfiguring " << queues_sp->size()
431+
<< " queues from domain " << d_name;
432+
361433
// Note: Queues must only be reconfigured AFTER ensuring that virtual
362434
// storage has been created for any new AppIds. This is done by the
363435
// 'QueueEngine::afterAppIdRegistered' method, which is invoked on the
@@ -370,19 +442,7 @@ int Domain::configure(bsl::ostream& errorDescription,
370442
// that it happens after 'onDomainReconfigured'; since implementation
371443
// of 'Queue::configure' dispatches to the Queue dispatcher thread, it
372444
// will also happen after completion of 'afterAppIdRegistered' above.
373-
BALL_LOG_INFO << "Reconfiguring " << d_queues.size()
374-
<< " queues from domain " << d_name;
375-
376-
QueueMap::iterator it = d_queues.begin();
377-
for (; it != d_queues.end(); it++) {
378-
bsl::function<int()> reconfigureQueueFn = bdlf::BindUtil::bind(
379-
&mqbi::Queue::configure,
380-
it->second.get(),
381-
static_cast<bsl::ostream*>(0), // errorDescription_p
382-
true, // isReconfigure
383-
false); // wait
384-
d_dispatcher_p->execute(reconfigureQueueFn, cluster());
385-
}
445+
d_dispatcher_p->execute(QueueReconfigureFunctor(queues_sp), cluster());
386446
}
387447
// 'wait==false', so the result of reconfiguration is not known
388448

@@ -450,7 +510,7 @@ void Domain::openQueue(
450510
const bmqp_ctrlmsg::QueueHandleParameters& handleParameters,
451511
const mqbi::Domain::OpenQueueCallback& callback)
452512
{
453-
// will execute in DomainManager's IO requester thread
513+
// Thread: DomainManager's IO requester thread
454514
// (TBD: for now, in client-session or cluster dispatcher thread)
455515

456516
// PRECONDITIONS
@@ -502,7 +562,7 @@ void Domain::openQueue(
502562

503563
int Domain::registerQueue(const bsl::shared_ptr<mqbi::Queue>& queueSp)
504564
{
505-
// executed by the associated CLUSTER's DISPATCHER thread
565+
// Thread: CLUSTER DISPATCHER
506566

507567
// PRECONDITIONS
508568
BSLS_ASSERT_SAFE(d_cluster_sp->inDispatcherThread());
@@ -555,7 +615,7 @@ int Domain::registerQueue(const bsl::shared_ptr<mqbi::Queue>& queueSp)
555615

556616
void Domain::unregisterQueue(mqbi::Queue* queue)
557617
{
558-
// executed by the associated CLUSTER's DISPATCHER thread
618+
// Thread: CLUSTER DISPATCHER
559619

560620
// PRECONDITIONS
561621
BSLS_ASSERT_SAFE(d_cluster_sp->inDispatcherThread());
@@ -597,7 +657,7 @@ void Domain::unregisterQueue(mqbi::Queue* queue)
597657
int Domain::processCommand(mqbcmd::DomainResult* result,
598658
const mqbcmd::DomainCommand& command)
599659
{
600-
// executed by *any* thread
660+
// Thread: ANY
601661

602662
if (command.isPurgeValue()) {
603663
// Some queues might be inactive. They don't have associated
@@ -875,7 +935,7 @@ void Domain::loadAllQueues(bsl::vector<bmqt::Uri>* out) const
875935
void Domain::loadRoutingConfiguration(
876936
bmqp_ctrlmsg::RoutingConfiguration* config) const
877937
{
878-
// executed by the associated CLUSTER's DISPATCHER thread
938+
// Thread: CLUSTER DISPATCHER
879939

880940
// PRECONDITIONS
881941
BSLS_ASSERT_SAFE(d_cluster_sp->inDispatcherThread());

src/groups/mqb/mqbblp/mqbblp_domain.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2015-2023 Bloomberg Finance L.P.
1+
// Copyright 2026 Bloomberg Finance L.P.
22
// SPDX-License-Identifier: Apache-2.0
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");

0 commit comments

Comments
 (0)