MB-19249: Address possible data races in ConnHandler context 70/62970/6
authorabhinavdangeti <abhinav@couchbase.com>
Mon, 5 Oct 2015 22:55:34 +0000 (15:55 -0700)
committerChiyoung Seo <chiyoung@couchbase.com>
Sat, 23 Apr 2016 01:04:26 +0000 (01:04 +0000)
WARNING: ThreadSanitizer: data race (pid=2443)

  Read of size 1 at 0x7d5000016a58 by thread T10:
    #0 ConnHandler::doDisconnect() /home/abhinav/couchbase/ep-engine/src/tapconnection.h:375 (ep.so+0x000000058416)
    #1 ExecutorThread::run() /home/abhinav/couchbase/ep-engine/src/executorthread.cc:112 (ep.so+0x0000000f87f6)
    #2 launch_executor_thread(void*) /home/abhinav/couchbase/ep-engine/src/executorthread.cc:33 (ep.so+0x0000000f8395)
    #3 platform_thread_wrap /home/abhinav/couchbase/platform/src/cb_pthreads.c:23 (libplatform.so.0.1.0+0x000000003d31)

  Previous write of size 1 at 0x7d5000016a58 by main thread:
    [failed to restore the stack]

  Location is heap block of size 480 at 0x7d5000016a00 allocated by main thread:
    #0 operator new(unsigned long) <null>:0 (engine_testapp+0x00000005084d)
    #1 DcpConnMap::newConsumer(void const*, std::string const&) /home/abhinav/couchbase/ep-engine/src/connmap.cc:969 (ep.so+0x000000048384)
    #2 EventuallyPersistentEngine::dcpOpen(void const*, unsigned int, unsigned int, unsigned int, void*, unsigned short) /home/abhinav/couchbase/ep-engine/src/ep_engine.cc:6189 (ep.so+0x0000000d3668)
    #3 EvpDcpOpen(engine_interface*, void const*, unsigned int, unsigned int, unsigned int, void*, unsigned short) /home/abhinav/couchbase/ep-engine/src/ep_engine.cc:1494 (ep.so+0x0000000b4e5f)
    #4 mock_dcp_open(engine_interface*, void const*, unsigned int, unsigned int, unsigned int, void*, unsigned short) /home/abhinav/couchbase/memcached/programs/engine_testapp/engine_testapp.cc:488 (engine_testapp+0x0000000bb015)
    #5 test_dcp_consumer_flow_control_aggressive(engine_interface*, engine_interface_v1*) /home/abhinav/couchbase/ep-engine/tests/ep_testsuite.cc:3826 (ep_testsuite.so+0x00000006ecfd)
    #6 execute_test(test, char const*, char const*) /home/abhinav/couchbase/memcached/programs/engine_testapp/engine_testapp.cc:1090 (engine_testapp+0x0000000b946c)
    #7 __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287 (libc.so.6+0x000000021ec4)

Change-Id: Id5223e93c416e5e5287c137d561aea1e453cbd41
Reviewed-on: http://review.couchbase.org/62970
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
src/tapconnection.h

index 2884520..239fd87 100644 (file)
@@ -269,11 +269,11 @@ public:
     void releaseReference(bool force = false);
 
     void setSupportAck(bool ack) {
-        supportAck = ack;
+        supportAck.store(ack);
     }
 
     bool supportsAck() const {
-        return supportAck;
+        return supportAck.load();
     }
 
     void setSupportCheckpointSync(bool checkpointSync) {
@@ -302,10 +302,10 @@ public:
 
     virtual void addStats(ADD_STAT add_stat, const void *c) {
         addStat("type", getType(), add_stat, c);
-        addStat("created", created, add_stat, c);
-        addStat("connected", connected, add_stat, c);
-        addStat("pending_disconnect", disconnect, add_stat, c);
-        addStat("supports_ack", supportAck, add_stat, c);
+        addStat("created", created.load(), add_stat, c);
+        addStat("connected", connected.load(), add_stat, c);
+        addStat("pending_disconnect", disconnect.load(), add_stat, c);
+        addStat("supports_ack", supportAck.load(), add_stat, c);
         addStat("reserved", reserved.load(), add_stat, c);
 
         if (numDisconnects > 0) {
@@ -367,19 +367,19 @@ public:
         if (!s) {
             ++numDisconnects;
         }
-        connected = s;
+        connected.store(s);
     }
 
     bool isConnected() {
-        return connected;
+        return connected.load();
     }
 
     bool doDisconnect() {
-        return disconnect;
+        return disconnect.load();
     }
 
     virtual void setDisconnect(bool val) {
-        disconnect = val;
+        disconnect.store(val);
     }
 
     static std::string getAnonName() {
@@ -417,16 +417,16 @@ private:
     hrtime_t connToken;
 
     //! Connection creation time
-    rel_time_t created;
+    AtomicValue<rel_time_t> created;
 
     //! The last time this connection's step function was called
     AtomicValue<rel_time_t> lastWalkTime;
 
     //! Should we disconnect as soon as possible?
-    bool disconnect;
+    AtomicValue<bool> disconnect;
 
     //! Is this tap conenction connected?
-    bool connected;
+    AtomicValue<bool> connected;
 
     //! Number of times this connection was disconnected
     AtomicValue<size_t> numDisconnects;
@@ -435,7 +435,7 @@ private:
     rel_time_t expiryTime;
 
     //! Whether or not this connection supports acking
-    bool supportAck;
+    AtomicValue<bool> supportAck;
 
     //! A counter used to generate unique names
     static AtomicValue<uint64_t> counter_;