Fixed memory leak: removed Binding and ExchangeBinding.

These classes unbind a deleted queue from any Exchanges.

But Exchanges hold shared_ptr<Queue>, so queues never deleted while
the exchange exists. Moreover queue-binding form a shared_ptr cycle
causing a leak.

Raised QPID-438 for the remaining problem: destroyed queues are never
unbound or deleted


git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/trunk@523857 13f79535-47bb-0310-9956-ffa450edef68

10 files changed, 5 insertions, 155 deletions

diff --git a/qpid/cpp/lib/broker/Binding.h b/qpid/cpp/lib/broker/Binding.h
deleted file mode 100644
index 16ca223208..0000000000
--- a/qpid/cpp/lib/broker/Binding.h
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- * 
- *   http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-#ifndef _Binding_
-#define _Binding_
-
-#include <FieldTable.h>
-
-namespace qpid {
-    namespace broker {
-        class Binding{
-        public:
-            virtual void cancel() = 0;
-            virtual ~Binding(){}
-        };
-    }
-}
-
-
-#endif
-
diff --git a/qpid/cpp/lib/broker/BrokerQueue.cpp b/qpid/cpp/lib/broker/BrokerQueue.cpp
index b65e8e3a9a..4b0ed6111c 100644
--- a/qpid/cpp/lib/broker/BrokerQueue.cpp
+++ b/qpid/cpp/lib/broker/BrokerQueue.cpp
@@ -50,16 +50,7 @@ Queue::Queue(const string& _name, uint32_t _autodelete,
     if(autodelete) lastUsed = now()/TIME_MSEC;
 }
 
-Queue::~Queue(){
-    for(Binding* b = bindings.front(); !bindings.empty(); b = bindings.front()){
-        b->cancel();
-        bindings.pop();
-    }
-}
-
-void Queue::bound(Binding* b){
-    bindings.push(b);
-}
+Queue::~Queue(){}
 
 void Queue::deliver(Message::shared_ptr& msg){
     enqueue(0, msg, 0);
diff --git a/qpid/cpp/lib/broker/BrokerQueue.h b/qpid/cpp/lib/broker/BrokerQueue.h
index 12f5815027..45cf317037 100644
--- a/qpid/cpp/lib/broker/BrokerQueue.h
+++ b/qpid/cpp/lib/broker/BrokerQueue.h
@@ -26,7 +26,6 @@
 #include <queue>
 #include <boost/shared_ptr.hpp>
 #include <amqp_types.h>
-#include <Binding.h>
 #include <ConnectionToken.h>
 #include <Consumer.h>
 #include <BrokerMessage.h>
@@ -54,7 +53,6 @@ namespace qpid {
          */
         class Queue{
             typedef std::vector<Consumer*> Consumers;
-            typedef std::queue<Binding*> Bindings;
             typedef std::queue<Message::shared_ptr> Messages;
             
             const string name;
@@ -62,7 +60,6 @@ namespace qpid {
             MessageStore* const store;
             const ConnectionToken* const owner;
             Consumers consumers;
-            Bindings bindings;
             Messages messages;
             bool queueing;
             bool dispatching;
@@ -94,11 +91,6 @@ namespace qpid {
             void configure(const qpid::framing::FieldTable& settings);
             void destroy();
             /**
-             * Informs the queue of a binding that should be cancelled on
-             * destruction of the queue.
-             */
-            void bound(Binding* b);
-            /**
              * Delivers a message to the queue. Will record it as
              * enqueued if persistent then process it.
              */
diff --git a/qpid/cpp/lib/broker/DirectExchange.cpp b/qpid/cpp/lib/broker/DirectExchange.cpp
index c898ae8d7e..0661e8c365 100644
--- a/qpid/cpp/lib/broker/DirectExchange.cpp
+++ b/qpid/cpp/lib/broker/DirectExchange.cpp
@@ -19,7 +19,6 @@
  *
  */
 #include <DirectExchange.h>
-#include <ExchangeBinding.h>
 #include <iostream>
 
 using namespace qpid::broker;
@@ -30,13 +29,12 @@ DirectExchange::DirectExchange(const string& _name) : Exchange(_name) {
 
 }
 
-void DirectExchange::bind(Queue::shared_ptr queue, const string& routingKey, const FieldTable* args){
+void DirectExchange::bind(Queue::shared_ptr queue, const string& routingKey, const FieldTable*){
     Mutex::ScopedLock l(lock);
     std::vector<Queue::shared_ptr>& queues(bindings[routingKey]);
     std::vector<Queue::shared_ptr>::iterator i = find(queues.begin(), queues.end(), queue);
     if(i == queues.end()){
         bindings[routingKey].push_back(queue);
-        queue->bound(new ExchangeBinding(this, queue, routingKey, args));
     }
 }
 
diff --git a/qpid/cpp/lib/broker/ExchangeBinding.cpp b/qpid/cpp/lib/broker/ExchangeBinding.cpp
deleted file mode 100644
index bf2102414d..0000000000
--- a/qpid/cpp/lib/broker/ExchangeBinding.cpp
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- * 
- *   http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-#include <ExchangeBinding.h>
-#include <BrokerExchange.h>
-
-using namespace qpid::broker;
-using namespace qpid::framing;
-
-ExchangeBinding::ExchangeBinding(Exchange* _e, Queue::shared_ptr _q, const string& _key, const FieldTable* _args) : e(_e), q(_q), key(_key), args(_args){}
-
-void ExchangeBinding::cancel(){
-    e->unbind(q, key, args);
-    delete this;
-}
-
-ExchangeBinding::~ExchangeBinding(){
-}
diff --git a/qpid/cpp/lib/broker/ExchangeBinding.h b/qpid/cpp/lib/broker/ExchangeBinding.h
deleted file mode 100644
index 2afaa89552..0000000000
--- a/qpid/cpp/lib/broker/ExchangeBinding.h
+++ /dev/null
@@ -1,48 +0,0 @@
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- * 
- *   http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-#ifndef _ExchangeBinding_
-#define _ExchangeBinding_
-
-#include <Binding.h>
-#include <FieldTable.h>
-#include <BrokerQueue.h>
-
-namespace qpid {
-    namespace broker {
-        class Exchange;
-        class Queue;
-
-        class ExchangeBinding : public virtual Binding{
-            Exchange* e;
-            Queue::shared_ptr q;
-            const string key;
-            const qpid::framing::FieldTable* args;
-        public:
-            ExchangeBinding(Exchange* _e, Queue::shared_ptr _q, const string& _key, const qpid::framing::FieldTable* _args);
-            virtual void cancel();
-            virtual ~ExchangeBinding();
-        };
-    }
-}
-
-
-#endif
-
diff --git a/qpid/cpp/lib/broker/FanOutExchange.cpp b/qpid/cpp/lib/broker/FanOutExchange.cpp
index 48afcc20d5..b487593efd 100644
--- a/qpid/cpp/lib/broker/FanOutExchange.cpp
+++ b/qpid/cpp/lib/broker/FanOutExchange.cpp
@@ -19,7 +19,6 @@
  *
  */
 #include <FanOutExchange.h>
-#include <ExchangeBinding.h>
 #include <algorithm>
 
 using namespace qpid::broker;
@@ -28,13 +27,12 @@ using namespace qpid::sys;
 
 FanOutExchange::FanOutExchange(const std::string& _name) : Exchange(_name) {}
 
-void FanOutExchange::bind(Queue::shared_ptr queue, const string& routingKey, const FieldTable* args){
+void FanOutExchange::bind(Queue::shared_ptr queue, const string& /*routingKey*/, const FieldTable* /*args*/){
     Mutex::ScopedLock locker(lock);
     // Add if not already present.
     Queue::vector::iterator i = std::find(bindings.begin(), bindings.end(), queue);
     if (i == bindings.end()) {
         bindings.push_back(queue);
-        queue->bound(new ExchangeBinding(this, queue, routingKey, args));
     }
 }
 
@@ -43,8 +41,6 @@ void FanOutExchange::unbind(Queue::shared_ptr queue, const string& /*routingKey*
     Queue::vector::iterator i = std::find(bindings.begin(), bindings.end(), queue);
     if (i != bindings.end()) {
         bindings.erase(i);
-        // TODO aconway 2006-09-14: What about the ExchangeBinding object?
-        // Don't we have to verify routingKey/args match?
     }
 }
 
diff --git a/qpid/cpp/lib/broker/HeadersExchange.cpp b/qpid/cpp/lib/broker/HeadersExchange.cpp
index acd344725a..3ef0cc0446 100644
--- a/qpid/cpp/lib/broker/HeadersExchange.cpp
+++ b/qpid/cpp/lib/broker/HeadersExchange.cpp
@@ -19,7 +19,6 @@
  *
  */
 #include <HeadersExchange.h>
-#include <ExchangeBinding.h>
 #include <Value.h>
 #include <QpidError.h>
 #include <algorithm>
@@ -43,14 +42,13 @@ namespace {
 
 HeadersExchange::HeadersExchange(const string& _name) : Exchange(_name) { }
 
-void HeadersExchange::bind(Queue::shared_ptr queue, const string& routingKey, const FieldTable* args){
+void HeadersExchange::bind(Queue::shared_ptr queue, const string& /*routingKey*/, const FieldTable* args){
     Mutex::ScopedLock locker(lock);
     std::string what = args->getString("x-match");
     if (what != all && what != any) {
         THROW_QPID_ERROR(PROTOCOL_ERROR, "Invalid x-match value binding to headers exchange.");
     }
     bindings.push_back(Binding(*args, queue));
-    queue->bound(new ExchangeBinding(this, queue, routingKey, args));
 }
 
 void HeadersExchange::unbind(Queue::shared_ptr queue, const string& /*routingKey*/, const FieldTable* args){
diff --git a/qpid/cpp/lib/broker/Makefile.am b/qpid/cpp/lib/broker/Makefile.am
index 68649c2b28..bc3d2089c2 100644
--- a/qpid/cpp/lib/broker/Makefile.am
+++ b/qpid/cpp/lib/broker/Makefile.am
@@ -42,8 +42,6 @@ libqpidbroker_la_SOURCES =			\
   DeliveryRecord.h				\
   DirectExchange.cpp				\
   DirectExchange.h				\
-  ExchangeBinding.cpp				\
-  ExchangeBinding.h				\
   ExchangeRegistry.cpp				\
   ExchangeRegistry.h				\
   FanOutExchange.cpp				\
diff --git a/qpid/cpp/lib/broker/TopicExchange.cpp b/qpid/cpp/lib/broker/TopicExchange.cpp
index 3ebb3c8c56..796d3cea02 100644
--- a/qpid/cpp/lib/broker/TopicExchange.cpp
+++ b/qpid/cpp/lib/broker/TopicExchange.cpp
@@ -19,7 +19,6 @@
  *
  */
 #include <TopicExchange.h>
-#include <ExchangeBinding.h>
 #include <algorithm>
 
 using namespace qpid::broker;
@@ -118,11 +117,10 @@ bool TopicPattern::match(const Tokens& target)  const
 
 TopicExchange::TopicExchange(const string& _name) : Exchange(_name) { }
 
-void TopicExchange::bind(Queue::shared_ptr queue, const string& routingKey, const FieldTable* args){
+void TopicExchange::bind(Queue::shared_ptr queue, const string& routingKey, const FieldTable* /*args*/){
     Monitor::ScopedLock l(lock);
     TopicPattern routingPattern(routingKey);
     bindings[routingPattern].push_back(queue);
-    queue->bound(new ExchangeBinding(this, queue, routingKey, args));
 }
 
 void TopicExchange::unbind(Queue::shared_ptr queue, const string& routingKey, const FieldTable* /*args*/){