From: Dave Rigby <daver@couchbase.com>
Date: Tue, 19 Apr 2016 10:38:37 +0000 (+0100)
Subject: MB-19273: Fix data race on PassiveStream::buffer.{bytes,items}
X-Git-Tag: v3.1.6~40
X-Git-Url: http://review.couchbase.org/gitweb?p=ep-engine.git;a=commitdiff_plain;h=78f946d67c577f1eb3d1a1824650f4167364c783

MB-19273: Fix data race on PassiveStream::buffer.{bytes,items}

As reported by ThreadSanitizer (see below), there is a dirty read on
{{buffer.items}} & {{buffer.bytes}} during stat writing, due to
PassiveStream::addStats not acquiring the {{bufMutex}} lock before
reading.

This appears benign as the stat is only user sent to users, not used
by ns_server etc for any calculation.

WARNING: ThreadSanitizer: data race (pid=28418)
  Read of size 8 at 0x7d5000018128 by main thread (mutexes: write M23810, write M969):
    #0 void STATWRITER_NAMESPACE::add_casted_stat<unsigned long>(char const*, unsigned long const&, void (*)(char const*, unsigned short, char const*, unsigned int, void const*), void const*) ep-engine/src/statwriter.h:47 (ep.so+0x0000000afe89)
    #1 PassiveStream::addStats(void (*)(char const*, unsigned short, char const*, unsigned int, void const*), void const*) ep-engine/src/dcp-stream.cc:1512 (ep.so+0x0000002a04ba)
    #2 DcpConsumer::addStats(void (*)(char const*, unsigned short, char const*, unsigned int, void const*), void const*) ep-engine/src/dcp-consumer.cc:555 (ep.so+0x00000026e867)

  Previous write of size 8 at 0x7d5000018128 by thread T18 (mutexes: write M23762):
    #0 PassiveStream::processBufferedMessages(unsigned int&) ep-engine/src/dcp-stream.cc:1311 (ep.so+0x00000029e196)
    #1 DcpConsumer::processBufferedItems() ep-engine/src/dcp-consumer.cc:599 (ep.so+0x0000002647e9)
    #2 Processer::run() ep-engine/src/dcp-consumer.cc:48 (ep.so+0x0000002643ef)

Change-Id: I443e85d59ffda3827b670e747794b3fcb69c4f7c
Reviewed-on: http://review.couchbase.org/63026
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
---

diff --git a/src/dcp-stream.cc b/src/dcp-stream.cc
index a90fd38..586a9c4 100644
--- a/src/dcp-stream.cc
+++ b/src/dcp-stream.cc
@@ -1506,10 +1506,17 @@ void PassiveStream::addStats(ADD_STAT add_stat, const void *c) {
 
     const int bsize = 128;
     char buf[bsize];
+    size_t buffer_bytes;
+    size_t buffer_items;
+    {
+        LockHolder lh(buffer.bufMutex);
+        buffer_bytes = buffer.bytes;
+        buffer_items = buffer.items;
+    }
     snprintf(buf, bsize, "%s:stream_%d_buffer_items", name_.c_str(), vb_);
-    add_casted_stat(buf, buffer.items, add_stat, c);
+    add_casted_stat(buf, buffer_items, add_stat, c);
     snprintf(buf, bsize, "%s:stream_%d_buffer_bytes", name_.c_str(), vb_);
-    add_casted_stat(buf, buffer.bytes, add_stat, c);
+    add_casted_stat(buf, buffer_bytes, add_stat, c);
     snprintf(buf, bsize, "%s:stream_%d_items_ready", name_.c_str(), vb_);
     add_casted_stat(buf, itemsReady.load() ? "true" : "false", add_stat, c);
     snprintf(buf, bsize, "%s:stream_%d_last_received_seqno", name_.c_str(), vb_);