From 5379465e0b2af3a0e4624e60769951ac703bc475 Mon Sep 17 00:00:00 2001
From: Alan Conway <aconway@apache.org>
Date: Fri, 8 Jun 2012 15:24:39 +0000
Subject: QPID-3603: Fix race condition causing sporadic crash in
 BrokerReplicator::initializeBridge

The BrokerReplicator was being deleted before all outstanding
initializeBridge calls had been processed by the IO thread.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1348115 13f79535-47bb-0310-9956-ffa450edef68
---
 qpid/cpp/src/qpid/broker/Broker.cpp       | 2 +-
 qpid/cpp/src/qpid/ha/Backup.cpp           | 3 +++
 qpid/cpp/src/qpid/ha/BrokerReplicator.cpp | 9 +++++++--
 qpid/cpp/src/qpid/ha/BrokerReplicator.h   | 8 ++++++--
 4 files changed, 17 insertions(+), 5 deletions(-)

(limited to 'qpid/cpp/src')

diff --git a/qpid/cpp/src/qpid/broker/Broker.cpp b/qpid/cpp/src/qpid/broker/Broker.cpp
index 6745204043..ed24db14ad 100644
--- a/qpid/cpp/src/qpid/broker/Broker.cpp
+++ b/qpid/cpp/src/qpid/broker/Broker.cpp
@@ -350,7 +350,7 @@ Broker::Broker(const Broker::Options& conf) :
         knownBrokers.push_back(Url(conf.knownHosts));
     }
 
-    } catch (const std::exception& /*e*/) {
+    } catch (const std::exception&) {
         finalize();
         throw;
     }
diff --git a/qpid/cpp/src/qpid/ha/Backup.cpp b/qpid/cpp/src/qpid/ha/Backup.cpp
index 158bdd927d..44fb098e79 100644
--- a/qpid/cpp/src/qpid/ha/Backup.cpp
+++ b/qpid/cpp/src/qpid/ha/Backup.cpp
@@ -86,11 +86,14 @@ void Backup::initialize(const Url& brokers) {
     link = result.first;
     link->setUrl(url);
     replicator.reset(new BrokerReplicator(haBroker, link));
+    replicator->initialize();
     broker.getExchanges().registerExchange(replicator);
 }
 
 Backup::~Backup() {
     if (link) link->close();
+    // FIXME aconway 2012-05-30: race: may have outstanding initializeBridge calls
+    // pointing to this.
     if (replicator.get()) broker.getExchanges().destroy(replicator->getName());
     replicator.reset();
 }
diff --git a/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp b/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp
index 7679078c40..0173495a00 100644
--- a/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp
+++ b/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp
@@ -172,7 +172,10 @@ BrokerReplicator::BrokerReplicator(HaBroker& hb, const boost::shared_ptr<Link>&
     : Exchange(QPID_CONFIGURATION_REPLICATOR),
       logPrefix(hb),
       haBroker(hb), broker(hb.getBroker()), link(l)
-{
+{}
+
+void BrokerReplicator::initialize() {
+    // Can't do this in the constructor because we need a shared_ptr to this.
     types::Uuid uuid(true);
     const std::string name(QPID_CONFIGURATION_REPLICATOR + ".bridge." + uuid.str());
     broker.getLinks().declare(
@@ -188,7 +191,9 @@ BrokerReplicator::BrokerReplicator(HaBroker& hb, const boost::shared_ptr<Link>&
         "",                 // excludes
         false,              // dynamic
         0,                  // sync?
-        boost::bind(&BrokerReplicator::initializeBridge, this, _1, _2)
+        // shared_ptr keeps this in memory until outstanding initializeBridge
+        // calls are run.
+        boost::bind(&BrokerReplicator::initializeBridge, shared_from_this(), _1, _2)
     );
 }
 
diff --git a/qpid/cpp/src/qpid/ha/BrokerReplicator.h b/qpid/cpp/src/qpid/ha/BrokerReplicator.h
index 57867587a9..f7466d6406 100644
--- a/qpid/cpp/src/qpid/ha/BrokerReplicator.h
+++ b/qpid/cpp/src/qpid/ha/BrokerReplicator.h
@@ -28,6 +28,7 @@
 #include "qpid/broker/Exchange.h"
 #include "qpid/types/Variant.h"
 #include <boost/shared_ptr.hpp>
+#include <boost/enable_shared_from_this.hpp>
 
 namespace qpid {
 
@@ -57,14 +58,17 @@ class QueueReplicator;
  * THREAD SAFE: Has no mutable state.
  *
  */
-class BrokerReplicator : public broker::Exchange
+class BrokerReplicator : public broker::Exchange,
+                         public boost::enable_shared_from_this<BrokerReplicator>
 {
   public:
     BrokerReplicator(HaBroker&, const boost::shared_ptr<broker::Link>&);
     ~BrokerReplicator();
-    std::string getType() const;
+
+    void initialize();
 
     // Exchange methods
+    std::string getType() const;
     bool bind(boost::shared_ptr<broker::Queue>, const std::string&, const framing::FieldTable*);
     bool unbind(boost::shared_ptr<broker::Queue>, const std::string&, const framing::FieldTable*);
     void route(broker::Deliverable&);
-- 
cgit v1.2.1