Fix[MQB]: unset queue raw pointer on config failure (#881)

678098 · web-flow · commit d12c59f7218f · 2025-08-14T00:31:01.000+01:00
diff --git a/src/groups/mqb/mqbblp/mqbblp_clusterqueuehelper.cpp b/src/groups/mqb/mqbblp/mqbblp_clusterqueuehelper.cpp
@@ -2168,13 +2168,21 @@ bsl::shared_ptr<mqbi::Queue> ClusterQueueHelper::createQueueFactory(
     bdlma::LocalSequentialAllocator<1024> localAllocator(d_allocator_p);
     bmqu::MemOutStream                    error(&localAllocator);
 
-    int rc = queueSp->configure(error,
+    int rc = queueSp->configure(&error,
                                 false,  // isReconfigure
                                 true);  // wait
 
+    /// `mqbi::Queue::configure` might have set a queue raw pointer in the
+    /// corresponding storage.  Make sure we unset this if we exit the scope
+    /// on error.
+    bdlb::ScopeExitAny queuePtrGuard(
+        bdlf::BindUtil::bindS(d_allocator_p,
+                              &mqbi::Storage::setQueue,
+                              queueSp->storage(),
+                              bsl::nullptr_t()));
+
     if (rc != 0) {
         // Queue.configure() failed.
-
         BMQ_LOGTHROTTLE_ERROR << "Failure configuring queue '"
                               << queueContext->uri() << "': " << error.str()
                               << ".";
@@ -2204,6 +2212,9 @@ bsl::shared_ptr<mqbi::Queue> ClusterQueueHelper::createQueueFactory(
             1);
     }
 
+    /// Success: no need to unset queue raw pointer.
+    queuePtrGuard.release();
+
     return queueSp;
 }
 
diff --git a/src/groups/mqb/mqbblp/mqbblp_domain.cpp b/src/groups/mqb/mqbblp/mqbblp_domain.cpp
@@ -348,9 +348,9 @@ int Domain::configure(bsl::ostream&           errorDescription,
             bsl::function<int()> reconfigureQueueFn = bdlf::BindUtil::bind(
                 &mqbi::Queue::configure,
                 it->second.get(),
-                bsl::ref(errorDescription),
-                true,    // isReconfigure
-                false);  // wait
+                bsl::nullptr_t(),  // errorDescription_p
+                true,              // isReconfigure
+                false);            // wait
             d_dispatcher_p->execute(reconfigureQueueFn, cluster());
         }
     }
diff --git a/src/groups/mqb/mqbblp/mqbblp_queue.cpp b/src/groups/mqb/mqbblp/mqbblp_queue.cpp
@@ -576,7 +576,7 @@ void Queue::onReplicatedBatch()
     }
 }
 
-int Queue::configure(bsl::ostream& errorDescription,
+int Queue::configure(bsl::ostream* errorDescription_p,
                      bool          isReconfigure,
                      bool          wait)
 {
@@ -588,7 +588,7 @@ int Queue::configure(bsl::ostream& errorDescription,
         bdlf::BindUtil::bind(&Queue::configureDispatched,
                              this,
                              (wait ? &result : NULL),
-                             (wait ? &errorDescription : NULL),
+                             (wait ? errorDescription_p : NULL),
                              isReconfigure),
         this);
     if (!wait) {
diff --git a/src/groups/mqb/mqbblp/mqbblp_queue.h b/src/groups/mqb/mqbblp/mqbblp_queue.h
@@ -194,16 +194,16 @@ class Queue BSLS_CPP11_FINAL : public mqbi::Queue {
     /// If `wait` is `true`, this method will not return until the operation
     /// has completed. The return value will be 0 if it succeeds, and
     /// nonzero if there was an error; in case of an error, the specified
-    /// `errorDescription` stream will be populated with a human readable
+    /// `errorDescription_p` stream will be populated with a human readable
     /// reason.
     ///
     /// If `wait` is `false`, this method will return 0 after scheduling the
-    /// operation on an unspecified thread, and `errorDescription` will be
+    /// operation on an unspecified thread, and `errorDescription_p` will be
     /// unmodified.
     ///
     /// THREAD: this method can be invoked only from cluster-dispatcher
     /// thread.
-    int configure(bsl::ostream& errorDescription,
+    int configure(bsl::ostream* errorDescription_p,
                   bool          isReconfigure,
                   bool          wait) BSLS_KEYWORD_OVERRIDE;
 
diff --git a/src/groups/mqb/mqbblp/mqbblp_queueenginetester.cpp b/src/groups/mqb/mqbblp/mqbblp_queueenginetester.cpp
@@ -471,7 +471,7 @@ void QueueEngineTester::init(const mqbconfm::Domain& domainConfig,
     // Register queue in domain
     bslma::ManagedPtr<mqbi::Queue> queueMp(d_mockQueue_sp.managedPtr());
 
-    rc = queueMp->configure(errorDescription,
+    rc = queueMp->configure(&errorDescription,
                             false,  // isReconfigure
                             true);  // wait
     BSLS_ASSERT_OPT(rc == 0);
diff --git a/src/groups/mqb/mqbblp/mqbblp_storagemanager.cpp b/src/groups/mqb/mqbblp/mqbblp_storagemanager.cpp
@@ -1172,7 +1172,6 @@ void StorageManager::setQueue(mqbi::Queue*     queue,
                                   queue));
 
     d_fileStores[partitionId]->dispatchEvent(queueEvent);
-    ;
 }
 
 void StorageManager::setQueueRaw(mqbi::Queue*     queue,
diff --git a/src/groups/mqb/mqbi/mqbi_queue.h b/src/groups/mqb/mqbi/mqbi_queue.h
@@ -732,15 +732,15 @@ class Queue : public DispatcherClient {
     /// If `wait` is `true`, this method will not return until the operation
     /// has completed. The return value will be 0 if it succeeds, and
     /// nonzero if there was an error; in case of an error, the specified
-    /// `errorDescription` stream will be populated with a human readable
+    /// `errorDescription_p` stream will be populated with a human readable
     /// reason.
     ///
     /// If `wait` is `false`, this method will return 0 after scheduling the
-    /// operation on an unspecified thread, and `errorDescription` will be
+    /// operation on an unspecified thread, and `errorDescription_p` will be
     /// unmodified.
     ///
     /// THREAD: this method can be called from any thread.
-    virtual int configure(bsl::ostream& errorDescription,
+    virtual int configure(bsl::ostream* errorDescription_p,
                           bool          isReconfigure,
                           bool          wait) = 0;
 
diff --git a/src/groups/mqb/mqbmock/mqbmock_queue.cpp b/src/groups/mqb/mqbmock/mqbmock_queue.cpp
@@ -104,7 +104,7 @@ void Queue::flush()
 
 // MANIPULATORS
 //   (virtual: mqbi::Queue)
-int Queue::configure(BSLA_UNUSED bsl::ostream& errorDescription,
+int Queue::configure(BSLA_UNUSED bsl::ostream* errorDescription_p,
                      BSLA_UNUSED bool          isReconfigure,
                      BSLA_UNUSED bool          wait)
 {
diff --git a/src/groups/mqb/mqbmock/mqbmock_queue.h b/src/groups/mqb/mqbmock/mqbmock_queue.h
@@ -191,7 +191,7 @@ class Queue : public mqbi::Queue {
 
     // MANIPULATORS
     //   (virtual: mqbi::Queue)
-    int configure(bsl::ostream& errorDescription,
+    int configure(bsl::ostream* errorDescription_p,
                   bool          isReconfigure,
                   bool          wait) BSLS_KEYWORD_OVERRIDE;
 
diff --git a/src/groups/mqb/mqbs/mqbs_filebackedstorage.cpp b/src/groups/mqb/mqbs/mqbs_filebackedstorage.cpp
@@ -735,6 +735,7 @@ int FileBackedStorage::gcExpiredMessages(
     bsls::Types::Int64*  configuredTtlValue,
     bsls::Types::Uint64  secondsFromEpoch)
 {
+    // Executed by QUEUE dispatcher thread
     BSLS_ASSERT_SAFE(d_store_p);
     BSLS_ASSERT_SAFE(latestMsgTimestampEpoch);
     BSLS_ASSERT_SAFE(configuredTtlValue);

Original file line number	Diff line number	Diff line change
`@@ -348,9 +348,9 @@ int Domain::configure(bsl::ostream& errorDescription,`
`348`	`348`	`bsl::function<int()> reconfigureQueueFn = bdlf::BindUtil::bind(`
`349`	`349`	`&mqbi::Queue::configure,`
`350`	`350`	`it->second.get(),`
`351`		`- bsl::ref(errorDescription),`
`352`		`- true, // isReconfigure`
`353`		`- false); // wait`
	`351`	`+ bsl::nullptr_t(), // errorDescription_p`
	`352`	`+ true, // isReconfigure`
	`353`	`+ false); // wait`
`354`	`354`	`d_dispatcher_p->execute(reconfigureQueueFn, cluster());`
`355`	`355`	`}`
`356`	`356`	`}`
Original file line number	Diff line number	Diff line change
`@@ -576,7 +576,7 @@ void Queue::onReplicatedBatch()`
`576`	`576`	`}`
`577`	`577`	`}`
`578`	`578`
`579`		`-int Queue::configure(bsl::ostream& errorDescription,`
	`579`	`+int Queue::configure(bsl::ostream* errorDescription_p,`
`580`	`580`	`bool isReconfigure,`
`581`	`581`	`bool wait)`
`582`	`582`	`{`
`@@ -588,7 +588,7 @@ int Queue::configure(bsl::ostream& errorDescription,`
`588`	`588`	`bdlf::BindUtil::bind(&Queue::configureDispatched,`
`589`	`589`	`this,`
`590`	`590`	`(wait ? &result : NULL),`
`591`		`- (wait ? &errorDescription : NULL),`
	`591`	`+ (wait ? errorDescription_p : NULL),`
`592`	`592`	`isReconfigure),`
`593`	`593`	`this);`
`594`	`594`	`if (!wait) {`
Original file line number	Diff line number	Diff line change
`@@ -1172,7 +1172,6 @@ void StorageManager::setQueue(mqbi::Queue* queue,`
`1172`	`1172`	`queue));`
`1173`	`1173`
`1174`	`1174`	`d_fileStores[partitionId]->dispatchEvent(queueEvent);`
`1175`		`- ;`
`1176`	`1175`	`}`
`1177`	`1176`
`1178`	`1177`	`void StorageManager::setQueueRaw(mqbi::Queue* queue,`
Original file line number	Diff line number	Diff line change
`@@ -104,7 +104,7 @@ void Queue::flush()`
`104`	`104`
`105`	`105`	`// MANIPULATORS`
`106`	`106`	`// (virtual: mqbi::Queue)`
`107`		`-int Queue::configure(BSLA_UNUSED bsl::ostream& errorDescription,`
	`107`	`+int Queue::configure(BSLA_UNUSED bsl::ostream* errorDescription_p,`
`108`	`108`	`BSLA_UNUSED bool isReconfigure,`
`109`	`109`	`BSLA_UNUSED bool wait)`
`110`	`110`	`{`
Original file line number	Diff line number	Diff line change
`@@ -735,6 +735,7 @@ int FileBackedStorage::gcExpiredMessages(`
`735`	`735`	`bsls::Types::Int64* configuredTtlValue,`
`736`	`736`	`bsls::Types::Uint64 secondsFromEpoch)`
`737`	`737`	`{`
	`738`	`+ // Executed by QUEUE dispatcher thread`
`738`	`739`	`BSLS_ASSERT_SAFE(d_store_p);`
`739`	`740`	`BSLS_ASSERT_SAFE(latestMsgTimestampEpoch);`
`740`	`741`	`BSLS_ASSERT_SAFE(configuredTtlValue);`