Skip to content

Commit 03384fe

Browse files
daverigbychiyoung
authored andcommitted
MB-15292: Make CouchbaseAtomic::load() atomic
Identified by ThreadSanitizer running ep_perfsuite.so: WARNING: ThreadSanitizer: data race (pid=51118) Atomic write of size 1 at 0x7d4400009d58 by main thread (mutexes: write M5599): #0 __tsan_atomic8_compare_exchange_val <null>:0 (engine_testapp+0x000000093f50) couchbase#1 CouchbaseAtomic<bool>::compare_exchange_strong(bool&, bool, memory_order) /root/couchbase-3.0/ep-engine/src/atomic.h:92 (ep.so+0x00000005575d) couchbase#2 Flusher::notifyFlushEvent() /root/couchbase-3.0/ep-engine/src/flusher.h:88 (ep.so+0x0000000c6fec) couchbase#3 EventuallyPersistentStore::queueDirty(RCPtr<VBucket>&, StoredValue*, LockHolder*, bool, bool, bool) /root/couchbase-3.0/ep-engine/src/ep.cc:2826 (ep.so+0x00000009c796) couchbase#4 EventuallyPersistentStore::add(Item const&, void const*) /root/couchbase-3.0/ep-engine/src/ep.cc:728 (ep.so+0x00000009f673) couchbase#5 EventuallyPersistentEngine::store(void const*, void*, unsigned long*, ENGINE_STORE_OPERATION, unsigned short) /root/couchbase-3.0/ep-engine/src/ep_engine.cc:2135 (ep.so+0x000000100980) #6 EvpStore(engine_interface*, void const*, void*, unsigned long*, ENGINE_STORE_OPERATION, unsigned short) /root/couchbase-3.0/ep-engine/src/ep_engine.cc:229 (ep.so+0x0000000fa21d) #7 mock_store /root/couchbase-3.0/memcached/programs/engine_testapp/engine_testapp.c:227 (engine_testapp+0x0000000ade2e) #8 storeCasVb11(engine_interface*, engine_interface_v1*, void const*, ENGINE_STORE_OPERATION, char const*, char const*, unsigned long, unsigned int, void**, unsigned long, unsigned short, unsigned int, unsigned char) /root/couchbase-3.0/ep-engine/tests/ep_test_apis.cc:654 (ep_perfsuite.so+0x000000018ec3) #9 perf_latency(engine_interface*, engine_interface_v1*, char const*) /root/couchbase-3.0/ep-engine/tests/ep_perfsuite.cc:104 (ep_perfsuite.so+0x0000000097e2) #10 perf_latency_baseline(engine_interface*, engine_interface_v1*) /root/couchbase-3.0/ep-engine/tests/ep_perfsuite.cc:169 (ep_perfsuite.so+0x0000000090b7) #11 execute_test /root/couchbase-3.0/memcached/programs/engine_testapp/engine_testapp.c:1042 (engine_testapp+0x0000000ab933) #12 main /root/couchbase-3.0/memcached/programs/engine_testapp/engine_testapp.c:1296 (engine_testapp+0x0000000a9a19) Previous read of size 1 at 0x7d4400009d58 by thread T20: #0 CouchbaseAtomic<bool>::load(memory_order) const /root/couchbase-3.0/ep-engine/src/atomic.h:79 (ep.so+0x00000005288c) couchbase#1 Flusher::canSnooze() /root/couchbase-3.0/ep-engine/src/flusher.h:104 (ep.so+0x00000018e92b) couchbase#2 Flusher::computeMinSleepTime() /root/couchbase-3.0/ep-engine/src/flusher.cc:239 (ep.so+0x00000018dc87) couchbase#3 Flusher::step(GlobalTask*) /root/couchbase-3.0/ep-engine/src/flusher.cc:187 (ep.so+0x00000018cb35) Change-Id: Ie32ca8fa4e662e1244362cbdb0cb2573f80665e2 Reviewed-on: http://review.couchbase.org/51968 Tested-by: buildbot <build@couchbase.com> Reviewed-by: Jim Walker <jim@couchbase.com> Reviewed-by: Chiyoung Seo <chiyoung@couchbase.com>
1 parent af66fd6 commit 03384fe

File tree

2 files changed

+40
-1
lines changed

2 files changed

+40
-1
lines changed

src/atomic.h

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class CouchbaseAtomic {
7676

7777
T load(memory_order sync = memory_order_acq_rel) const {
7878
(void) sync;
79-
return value;
79+
return ep_sync_fetch_and_add(const_cast<T*>(&value), T(0));
8080
}
8181

8282
void store(const T &newValue, memory_order sync = memory_order_acq_rel) {
@@ -163,6 +163,40 @@ class CouchbaseAtomic {
163163
volatile T value;
164164
};
165165

166+
template <>
167+
class CouchbaseAtomic<double> {
168+
typedef union doubleIntStore {
169+
double d;
170+
uint64_t i;
171+
} DoubleIntStore;
172+
173+
public:
174+
CouchbaseAtomic(const double& initial = 0) {
175+
store(initial);
176+
}
177+
178+
double load(memory_order sync = memory_order_acq_rel) const {
179+
(void) sync;
180+
DoubleIntStore rv;
181+
rv.i = ep_sync_fetch_and_add((uint64_t*)&value.i, 0);
182+
return rv.d;
183+
}
184+
185+
void store(const double& newValue, memory_order sync = memory_order_acq_rel) {
186+
(void) sync;
187+
DoubleIntStore input;
188+
input.d = newValue;
189+
(void)ep_sync_lock_test_and_set(&value.i, input.i);
190+
}
191+
192+
operator double() const {
193+
return load();
194+
}
195+
196+
private:
197+
volatile DoubleIntStore value;
198+
};
199+
166200
#endif
167201

168202
template <typename T>

tests/module_tests/atomic_test.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ static void testSetIfBigger() {
8282
cb_assert(x.load() == 924);
8383
}
8484

85+
static void testAtomicDouble(void) {
86+
AtomicValue<double> d = 991.5;
87+
cb_assert(d == 991.5);
88+
}
8589

8690
int testAtomicCompareExchangeStrong(void) {
8791
AtomicValue<bool> x(true);
@@ -125,5 +129,6 @@ int main() {
125129
testAtomicInt();
126130
testSetIfLess();
127131
testSetIfBigger();
132+
testAtomicDouble();
128133
return testAtomicCompareExchangeStrong();
129134
}

0 commit comments

Comments
 (0)