From 56490d4f25394ba3ed9e49141cfc9d01b1258719 Mon Sep 17 00:00:00 2001
From: James Harrison <james.harrison@couchbase.com>
Date: Tue, 19 Jan 2021 11:25:57 +0000
Subject: [PATCH] MB-43717: Fix race in BasicLinkedList::purgeTombstones

The list write lock must be held when updating the pausedPurgePoint to
avoid racing with updateListElem reading it.

 WARNING: ThreadSanitizer: data race (pid=17198)
   Read of size 8 at 0x7b4c00002d40 by thread T2 (mutexes: read M666668208119350224, write M838931151564315824, write M830768325700119856, write M825420198063385568):
     #0 boost::intrusive::list_iterator<boost::intrusive::mhtraits<OrderedStoredValue, boost::intrusive::list_member_hook<>, &OrderedStoredValue::seqno_hook>, false>::pointed_node() const <null> (libep.so+0x0000003c5dd5)
     #1 boost::intrusive::operator==(boost::intrusive::list_iterator<boost::intrusive::mhtraits<OrderedStoredValue, boost::intrusive::list_member_hook<>, &OrderedStoredValue::seqno_hook>, false> const&, boost::intrusive::list_iterator<boost::intrusive::mhtraits<OrderedStoredValue, boost::intrusive::list_member_hook<>, &OrderedStoredValue::seqno_hook>, false> const&) tlm/deps/boost.exploded/include/boost/intrusive/detail/list_iterator.hpp:119 (libep.so+0x0000003c230b)
     #2 BasicLinkedList::updateListElem(std::lock_guard<std::mutex>&, std::lock_guard<std::mutex>&, OrderedStoredValue&) ../kv_engine/engines/ep/src/linked_list.cc:82 (libep.so+0x0000003bf176)
     #3 EphemeralVBucket::modifySeqList(std::lock_guard<std::mutex>&, std::lock_guard<std::mutex>&, OrderedStoredValue&) ../kv_engine/engines/ep/src/ephemeral_vb.cc:932 (libep.so+0x000000327553)

...

   Previous write of size 8 at 0x7b4c00002d40 by thread T30:
     #0 boost::intrusive::list_iterator<boost::intrusive::mhtraits<OrderedStoredValue, boost::intrusive::list_member_hook<>, &OrderedStoredValue::seqno_hook>, false>::operator=(boost::intrusive::list_iterator<boost::intrusive::mhtraits<OrderedStoredValue, boost::intrusive::list_member_hook<>, &OrderedStoredValue::seqno_hook>, false> const&) tlm/deps/boost.exploded/include/boost/intrusive/detail/list_iterator.hpp:79 (libep.so+0x0000003c2436)
     #1 BasicLinkedList::purgeTombstones(long, std::function<bool (DocKey const&, long)>, std::function<bool ()>) ../kv_engine/engines/ep/src/linked_list.cc:360 (libep.so+0x0000003c03da)
     #2 EphemeralVBucket::purgeStaleItems(std::function<bool ()>) ../kv_engine/engines/ep/src/ephemeral_vb.cc:391 (libep.so+0x000000326f56)
     #3 EphemeralVBucket::StaleItemDeleter::visit(VBucket&) ../kv_engine/engines/ep/src/ephemeral_tombstone_purger.cc:211 (libep.so+0x000000324a5e)
     #4 KVBucket::pauseResumeVisit(PauseResumeVBVisitor&, KVBucketIface::Position&) ../kv_engine/engines/ep/src/kv_bucket.cc:2322 (libep.so+0x0000003831e8)
     #5 EphTombstoneStaleItemDeleter::run() ../kv_engine/engines/ep/src/ephemeral_tombstone_purger.cc:280 (libep.so+0x0000003211dc)

Change-Id: I5fd6d4bfeef5831420f9f6a472ba7bc817753c23
Reviewed-on: http://review.couchbase.org/c/kv_engine/+/143784
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
---
 engines/ep/src/linked_list.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/engines/ep/src/linked_list.cc b/engines/ep/src/linked_list.cc
index c8feb99916..e2633e7b29 100644
--- a/engines/ep/src/linked_list.cc
+++ b/engines/ep/src/linked_list.cc
@@ -357,6 +357,7 @@ size_t BasicLinkedList::purgeTombstones(
                 //  so next time purge is attempted it will resume from here (the
                 //  range lock "blocking" part of the requested seqno range may have
                 //  moved/gone)
+                std::lock_guard<std::mutex> writeGuard(getListWriteLock());
                 pausedPurgePoint = it;
             }
             // reached the end of the locked range, stop