MB-20645: Don't request stats from null DCP backfill manager 25/67025/3
authorDave Rigby <daver@couchbase.com>
Wed, 24 Aug 2016 10:54:13 +0000 (11:54 +0100)
committerDave Rigby <daver@couchbase.com>
Thu, 25 Aug 2016 08:00:37 +0000 (08:00 +0000)
If a DCP Producer has DcpProducer::addStats called on it after its
been disconnected (but before it's removed from the connMap) then we
end up dereferencing a null backfillMgr pointer.

Fix by adding a guard that the manager is valid before including its
stats.

Change-Id: Idc97b447090f5390054a9c40f207dae5494e63b9
Reviewed-on: http://review.couchbase.org/67025
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
src/dcp/producer.cc
tests/module_tests/dcp_test.cc

index 36994b6..de7cc29 100644 (file)
@@ -722,7 +722,11 @@ void DcpProducer::addStats(ADD_STAT add_stat, const void *c) {
             supportsCursorDropping ? "ELIGIBLE" : "NOT_ELIGIBLE",
             add_stat, c);
 
-    backfillMgr->addStats(this, add_stat, c);
+    // Possible that the producer has had its streams closed and hence doesn't
+    // have a backfill manager anymore.
+    if (backfillMgr) {
+        backfillMgr->addStats(this, add_stat, c);
+    }
 
     log.addStats(add_stat, c);
 
index f8b408b..d45d396 100644 (file)
@@ -592,6 +592,28 @@ TEST_F(ConnectionTest, test_update_of_last_message_time_in_consumer) {
     destroy_mock_cookie(cookie);
 }
 
+// Regression test for MB 20645 - ensure that a call to addStats after a
+// connection has been disconnected (and closeAllStreams called) doesn't crash.
+TEST_F(ConnectionTest, test_mb20645_stats_after_closeAllStreams) {
+    MockDcpConnMap connMap(*engine);
+    connMap.initialize(DCP_CONN_NOTIFIER);
+    const void *cookie = create_mock_cookie();
+    // Create a new Dcp producer
+    dcp_producer_t producer = connMap.newProducer(cookie, "test_producer",
+                                    /*notifyOnly*/false);
+
+    // Disconnect the producer connection
+    connMap.disconnect(cookie);
+
+    // Try to read stats. Shouldn't crash.
+    producer->addStats([](const char *key, const uint16_t klen,
+                          const char *val, const uint32_t vlen,
+                          const void *cookie) {}, nullptr);
+
+    destroy_mock_cookie(cookie);
+}
+
+
 class NotifyTest : public DCPTest {
 protected:
     void SetUp() {