VBSeqlistStats test: Fix locking ordering

The Visitor test class used to page out a key for VBSeqlistStats
leaves the collection manifest locked even when the visitor has
completed. This results in a lock-order inversion error later on from
TSan when the test later requests vbucket-details stats (once
http://review.couchbase.org/#/c/109734/ is applied):

  Mutex M1111398894660490488 acquired here while holding mutex M1100984080004130504 in main thread:
    #0 AnnotateRWLockAcquired <null> (libtsan.so.0+0x00000005b63d)
    ...
    #6 VBucket::getSyncWriteAcceptedCount() const kv_engine/engines/ep/src/vbucket.cc:304 (ep-engine_ep_unit_tests+0x000000e6891e)
    #7 VBucket::_addStats(bool, std::function<void (char const*, unsigned short, char const*, unsigned int, gsl::not_null<void const*>)> const&, void const*) kv_engine/engines/ep/src/vbucket.cc:2873 (ep-engine_ep_unit_tests+0x000000e6c098)
    ...
    #13 StatTest::get_stat[abi:cxx11](char const*) kv_engine/engines/ep/tests/module_tests/stats_test.cc:66 (ep-engine_ep_unit_tests+0x000000b99ea5)
    #14 EphemeralBucketStatTest_VBSeqlistStats_Test::TestBody() kv_engine/engines/ep/tests/module_tests/ephemeral_bucket_test.cc:92 (ep-engine_ep_unit_tests+0x0000008476d7)

  Mutex M1100984080004130504 previously acquired by the same thread here:
    #0 pthread_rwlock_rdlock <null> (libtsan.so.0+0x00000002953b)
    ...
    #7 Collections::VB::Manifest::lock() const kv_engine/engines/ep/src/collections/vbucket_manifest.h:679 (ep-engine_ep_unit_tests+0x00000084745a)
    #8 VBucket::lockCollections() const kv_engine/engines/ep/src/vbucket.h:756 (ep-engine_ep_unit_tests+0x00000084745a)
    #9 Visitor kv_engine/engines/ep/tests/module_tests/ephemeral_bucket_test.cc:73 (ep-engine_ep_unit_tests+0x00000084745a)
    #10 EphemeralBucketStatTest_VBSeqlistStats_Test::TestBody() kv_engine/engines/ep/tests/module_tests/ephemeral_bucket_test.cc:89 (ep-engine_ep_unit_tests+0x00000084745a)

Avoid this by using the same pattern used elsewhere in production code
- only acquire the collections manifest lock before/after the visit
(via setUpHashBucketVisit / tearDownHashBucketVisit).

Change-Id: I22039f316b5664cebe458a76f64cc863d79d25b1
Reviewed-on: http://review.couchbase.org/109932
Tested-by: Build Bot <[email protected]>
Reviewed-by: Ben Huddleston <[email protected]>

Author: daverigby

engines/ep/tests/module_tests/ephemeral_bucket_test.cc

@@ -69,9 +69,9 @@ TEST_F(EphemeralBucketStatTest, VBSeqlistStats) {
 
     // Test visitor which pages out our key
     struct Visitor : public HashTableVisitor {
-        Visitor(VBucket& vb, StoredDocKey key)
-            : vb(vb), key(key), readHandle(vb.lockCollections()) {
+        Visitor(VBucket& vb, StoredDocKey key) : vb(vb), key(key) {
         }
+
         bool visit(const HashTable::HashBucketLock& lh,
                    StoredValue& v) override {
             if (v.getKey() == key) {
@@ -80,6 +80,16 @@ TEST_F(EphemeralBucketStatTest, VBSeqlistStats) {
             }
             return true;
         }
+
+        void setUpHashBucketVisit() override {
+            // Need to lock collections before we visit each SV.
+            readHandle = vb.lockCollections();
+        }
+
+        void tearDownHashBucketVisit() override {
+            readHandle.unlock();
+        }
+
         VBucket& vb;
         StoredDocKey key;
         Collections::VB::Manifest::ReadHandle readHandle;