MB-30148: ep_testsuite: ensure Item is reserved before reading

TSan reports the follow use-after-free error:

    WARNING: ThreadSanitizer: heap-use-after-free (pid=5347)
      Read of size 1 at 0x7b7400040609 by main thread:
        #0 memcmp <null> (libtsan.so.0+0x000000043643)
        couchbase#1 check_key_value() kv_engine/engines/ep/tests/ep_testsuite_common.cc:468 (ep_testsuite.so+0x0000000975ad)
        couchbase#2 test_mem_stats kv_engine/engines/ep/tests/ep_testsuite.cc:2036 (ep_testsuite.so+0x00000002c5c6)
        couchbase#3 execute_test kv_engine/programs/engine_testapp/engine_testapp.cc:1102 (engine_testapp+0x00000041b0ba)
        couchbase#4 main kv_engine/programs/engine_testapp/engine_testapp.cc:1499 (engine_testapp+0x00000041c5b2)

      Previous write of size 8 at 0x7b7400040608 by thread T11 (mutexes: write M3231945710371016):
        #0 operator delete() <null> (libtsan.so.0+0x00000006a7b4)
        couchbase#1 Blob::operator delete() kv_engine/engines/ep/src/blob.h:124 (ep.so+0x0000001959c2)
        couchbase#2 Blob::Deleter::operator()(TaggedPtr<Blob>) kv_engine/engines/ep/src/blob.h:137 (ep.so+0x0000001959c2)
        couchbase#3 SingleThreadedRCPtr<Blob, TaggedPtr<Blob>, Blob::Deleter>::swap(TaggedPtr<Blob>) kv_engine/engines/ep/src/atomic.h:362 (ep.so+0x0000001959c2)
        couchbase#4 SingleThreadedRCPtr<Blob, TaggedPtr<Blob>, Blob::Deleter>::reset(TaggedPtr<Blob>) kv_engine/engines/ep/src/atomic.h:298 (ep.so+0x0000001959c2)
        couchbase#5 StoredValue::replaceValue(TaggedPtr<Blob>) kv_engine/engines/ep/src/stored-value.h:540 (ep.so+0x0000001959c2)
        couchbase#6 StoredValue::storeCompressedBuffer(cb::const_char_buffer) kv_engine/engines/ep/src/stored-value.cc:362 (ep.so+0x0000001959c2)
        #7 HashTable::storeCompressedBuffer(cb::const_char_buffer, StoredValue&) kv_engine/engines/ep/src/hash_table.cc:620 (ep.so+0x00000012be5c)
        #8 ItemCompressorVisitor::visit(HashTable::HashBucketLock const&, StoredValue&) kv_engine/engines/ep/src/item_compressor_visitor.cc:52 (ep.so+0x000000139780)
        #9 HashTable::pauseResumeVisit(HashTableVisitor&, HashTable::Position&) kv_engine/engines/ep/src/hash_table.cc:753 (ep.so+0x000000127b6c)
        #10 PauseResumeVBAdapter::visit(VBucket&) kv_engine/engines/ep/src/vb_visitors.cc:36 (ep.so+0x0000001a81b1)
        #11 KVBucket::pauseResumeVisit(PauseResumeVBVisitor&, KVBucketIface::Position&) kv_engine/engines/ep/src/kv_bucket.cc:2177 (ep.so+0x000000158ec9)
        #12 ItemCompressorTask::run() kv_engine/engines/ep/src/item_compressor.cc:70 (ep.so+0x000000138204)
        #13 ExecutorThread::run() kv_engine/engines/ep/src/executorthread.cc:146 (ep.so+0x00000011da74)
        #14 launch_executor_thread kv_engine/engines/ep/src/executorthread.cc:34 (ep.so+0x00000011e0ae)
        #15 CouchbaseThread::run() platform/src/cb_pthreads.cc:59 (libplatform_so.so.0.1.0+0x000000009cc9)
        #16 platform_thread_wrap platform/src/cb_pthreads.cc:72 (libplatform_so.so.0.1.0+0x000000009cc9)
        #17 <null> <null> (libtsan.so.0+0x000000024feb)

      Mutex M3231945710371016 is already destroyed.

The issue is in In check_key_value(); which uses get_item_info() to
retrieve the address and size of the value (Blob) of the given key
before checking it. get_item_info() doesn't retain a reference on the
underlying engine's Item and consequently Blob. As such it's not safe
to access the underlying Blob.

Fix by explicitly fetching the Item via get(), and retaining the
return value for the duration of accessing the Blob. This ensures the
ref-count is kept and so the Blob cannot be deleted while
check_key_value() is accessing it.

Change-Id: If6cefef4d988ca26c33e798ecc383e94478c474d
Reviewed-on: http://review.couchbase.org/95743
Well-Formed: Build Bot <[email protected]>
Reviewed-by: Tim Bradgate <[email protected]>
Reviewed-by: Sriram Ganesan <[email protected]>
Tested-by: Build Bot <[email protected]>

Author: daverigby

engines/ep/tests/ep_testsuite_common.cc

@@ -447,8 +447,15 @@ void destroy_buckets(std::vector<BucketHolder> &buckets) {
 void check_key_value(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
                      const char* key, const char* val, size_t vlen,
                      uint16_t vbucket) {
+    // Fetch item itself, to ensure we maintain a ref-count on the underlying
+    // Blob while comparing the key.
+    auto getResult = get(h, h1, NULL, key, vbucket);
+    checkeq(cb::engine_errc::success,
+            getResult.first,
+            "Failed to fetch document");
     item_info info;
-    check(get_item_info(h, h1, &info, key, vbucket), "checking key and value");
+    check(h1->get_item_info(h, getResult.second.get(), &info),
+          "Failed to get_item_info");
 
     cb::const_char_buffer payload;
     cb::compression::Buffer inflated;