From 4640a9ad6a32147320e1c527873184fd66186558 Mon Sep 17 00:00:00 2001
From: Matt Broadstone <mbroadstone@devonit.com>
Date: Fri, 6 Feb 2015 16:12:58 -0500
Subject: [PATCH] cleanup frame handlers on channel deletion

In cases where exchanges and queues are added and deleted during
an extended use of a QAmqpClient, the client should internally cleanup
the frame handlers registered for those objects. This patch does that
as well as providing two test cases verifying this behavior for both
QAmqpQueue and QAmqpExchange.
---
 src/qamqpchannel.cpp                           |  4 ++++
 src/qamqpchannel.h                             |  1 +
 src/qamqpchannel_p.h                           |  1 +
 src/qamqpclient.h                              |  1 +
 src/qamqpqueue.cpp                             |  7 +++++++
 tests/auto/qamqpexchange/tst_qamqpexchange.cpp | 18 ++++++++++++++++++
 tests/auto/qamqpqueue/tst_qamqpqueue.cpp       | 18 ++++++++++++++++++
 7 files changed, 50 insertions(+)

diff --git a/src/qamqpchannel.cpp b/src/qamqpchannel.cpp
index 5a1160a..a33dbd0 100644
--- a/src/qamqpchannel.cpp
+++ b/src/qamqpchannel.cpp
@@ -22,6 +22,10 @@ QAmqpChannelPrivate::QAmqpChannelPrivate(QAmqpChannel *q)
 
 QAmqpChannelPrivate::~QAmqpChannelPrivate()
 {
+    if (!client.isNull()) {
+        QAmqpClientPrivate *priv = client->d_func();
+        priv->methodHandlersByChannel[channelNumber].removeAll(this);
+    }
 }
 
 void QAmqpChannelPrivate::init(int channel, QAmqpClient *c)
diff --git a/src/qamqpchannel.h b/src/qamqpchannel.h
index ef14f09..7be66a5 100644
--- a/src/qamqpchannel.h
+++ b/src/qamqpchannel.h
@@ -58,6 +58,7 @@ protected:
     Q_PRIVATE_SLOT(d_func(), void _q_disconnected())
 
     friend class QAmqpClientPrivate;
+    friend class QAmqpExchangePrivate;
 };
 
 #endif  // QAMQPCHANNEL_H
diff --git a/src/qamqpchannel_p.h b/src/qamqpchannel_p.h
index 7a79945..372f386 100644
--- a/src/qamqpchannel_p.h
+++ b/src/qamqpchannel_p.h
@@ -6,6 +6,7 @@
 
 #define METHOD_ID_ENUM(name, id) name = id, name ## Ok
 
+class QAmqpChannel;
 class QAmqpClient;
 class QAmqpClientPrivate;
 class QAmqpChannelPrivate : public QAmqpMethodFrameHandler
diff --git a/src/qamqpclient.h b/src/qamqpclient.h
index 51b15a2..ffab502 100644
--- a/src/qamqpclient.h
+++ b/src/qamqpclient.h
@@ -112,6 +112,7 @@ private:
     Q_PRIVATE_SLOT(d_func(), void _q_disconnect())
 
     friend class QAmqpChannelPrivate;
+    friend class QAmqpQueuePrivate;
 
 };
 
diff --git a/src/qamqpqueue.cpp b/src/qamqpqueue.cpp
index 9d9b4b8..f429027 100644
--- a/src/qamqpqueue.cpp
+++ b/src/qamqpqueue.cpp
@@ -3,6 +3,8 @@
 #include <QDataStream>
 #include <QFile>
 
+#include "qamqpclient.h"
+#include "qamqpclient_p.h"
 #include "qamqpqueue.h"
 #include "qamqpqueue_p.h"
 #include "qamqpexchange.h"
@@ -21,6 +23,11 @@ QAmqpQueuePrivate::QAmqpQueuePrivate(QAmqpQueue *q)
 
 QAmqpQueuePrivate::~QAmqpQueuePrivate()
 {
+    if (!client.isNull()) {
+        QAmqpClientPrivate *priv = client->d_func();
+        priv->contentHandlerByChannel[channelNumber].removeAll(this);
+        priv->bodyHandlersByChannel[channelNumber].removeAll(this);
+    }
 }
 
 bool QAmqpQueuePrivate::_q_method(const QAmqpMethodFrame &frame)
diff --git a/tests/auto/qamqpexchange/tst_qamqpexchange.cpp b/tests/auto/qamqpexchange/tst_qamqpexchange.cpp
index 6677af8..527602b 100644
--- a/tests/auto/qamqpexchange/tst_qamqpexchange.cpp
+++ b/tests/auto/qamqpexchange/tst_qamqpexchange.cpp
@@ -26,6 +26,7 @@ private Q_SLOTS:
     void confirmsSupport();
     void confirmDontLoseMessages();
     void passiveDeclareNotFound();
+    void cleanupOnDeletion();
 
 private:
     QScopedPointer<QAmqpClient> client;
@@ -199,5 +200,22 @@ void tst_QAMQPExchange::passiveDeclareNotFound()
     QCOMPARE(nonExistentExchange->error(), QAMQP::NotFoundError);
 }
 
+void tst_QAMQPExchange::cleanupOnDeletion()
+{
+    // create, declare, and close the wrong way
+    QAmqpExchange *exchange = client->createExchange("test-deletion");
+    exchange->declare();
+    QVERIFY(waitForSignal(exchange, SIGNAL(declared())));
+    exchange->close();
+    exchange->deleteLater();
+
+    // now create, declare, and close the right way
+    exchange = client->createExchange("test-deletion");
+    exchange->declare();
+    QVERIFY(waitForSignal(exchange, SIGNAL(declared())));
+    exchange->close();
+    QVERIFY(waitForSignal(exchange, SIGNAL(closed())));
+}
+
 QTEST_MAIN(tst_QAMQPExchange)
 #include "tst_qamqpexchange.moc"
diff --git a/tests/auto/qamqpqueue/tst_qamqpqueue.cpp b/tests/auto/qamqpqueue/tst_qamqpqueue.cpp
index b27a4ac..f4ee397 100644
--- a/tests/auto/qamqpqueue/tst_qamqpqueue.cpp
+++ b/tests/auto/qamqpqueue/tst_qamqpqueue.cpp
@@ -46,6 +46,7 @@ private Q_SLOTS:
     void tableFieldDataTypes();
     void messageProperties();
     void emptyMessage();
+    void cleanupOnDeletion();
 
 private:
     QScopedPointer<QAmqpClient> client;
@@ -653,5 +654,22 @@ void tst_QAMQPQueue::emptyMessage()
     QVERIFY(message.payload().isEmpty());
 }
 
+void tst_QAMQPQueue::cleanupOnDeletion()
+{
+    // create, declare, and close the wrong way
+    QAmqpQueue *queue = client->createQueue("test-deletion");
+    queue->declare();
+    QVERIFY(waitForSignal(queue, SIGNAL(declared())));
+    queue->close();
+    queue->deleteLater();
+
+    // now create, declare, and close the right way
+    queue = client->createQueue("test-deletion");
+    queue->declare();
+    QVERIFY(waitForSignal(queue, SIGNAL(declared())));
+    queue->close();
+    QVERIFY(waitForSignal(queue, SIGNAL(closed())));
+}
+
 QTEST_MAIN(tst_QAMQPQueue)
 #include "tst_qamqpqueue.moc"