MB-21379: Ensure vals data structure is thread safe in ep_testsuite

When developing additional stats test for ep-engine_perfsuite, the
following data race was uncovered.

The patch ensures that accesses to vals are protected with a mutex.
Also get_stat and get_histo_stat can only be called one at a time as
they use the three global variables (requested_stat_name,
actual_stat_value and histogram_stat_int_value).  Therefore a secondary
mutex is required to enforce this.

It has been confirmed that with this patch the ThreadSanitizer issue
no longer occurs.

30: WARNING: ThreadSanitizer: data race (pid=42065)
30:   Read of size 8 at 0x7d1800060a38 by thread T18 (mutexes: write M1704):
30:     #0 std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::_S_right(std::_Rb_tree_node_base*) <null> (engine_testapp+0x00000049c503)
30:     #1 std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::_M_lower_bound(std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >*, std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) <null> (engine_testapp+0x0000004a30db)
30:     #2 std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::lower_bound(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) <null> (engine_testapp+0x00000049c5f6)
30:     #3 std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::lower_bound(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) <null> (engine_testapp+0x000000499047)
30:     #4 std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::operator[](std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) <null> (engine_testapp+0x00000048e3ba)
30:     #5 add_stats /home/owend/master/ep-engine/tests/ep_test_apis.cc:236 (ep_perfsuite.so+0x00000008f5b9)
30:     #6 STATWRITER_NAMESPACE::add_casted_stat(char const*, char const*, void (*)(char const*, unsigned short, char const*, unsigned int, void const*), void const*) /home/owend/master/ep-engine/src/statwriter.h:39 (ep.so+0x0000000e5673)
30:     #7 StatCheckpointVisitor::addCheckpointStat(void const*, void (*)(char const*, unsigned short, char const*, unsigned int, void const*), EventuallyPersistentStore*, RCPtr<VBucket>&) <null> (ep.so+0x0000001c120c)
30:     #8 StatCheckpointVisitor::visitBucket(RCPtr<VBucket>&) <null> (ep.so+0x0000001c10e5)
30:     #9 EventuallyPersistentStore::visit(VBucketVisitor&) /home/owend/master/ep-engine/src/ep.cc:3796 (ep.so+0x000000174559)
30:     #10 StatCheckpointTask::run() <null> (ep.so+0x0000001c155b)
30:     #11 ExecutorThread::run() /home/owend/master/ep-engine/src/executorthread.cc:113 (ep.so+0x0000001ea96d)
30:     #12 launch_executor_thread /home/owend/master/ep-engine/src/executorthread.cc:31 (ep.so+0x0000001ea0d0)
30:     #13 CouchbaseThread::run() /home/owend/master/platform/src/cb_pthreads.cc:58 (libplatform_so.so.0.1.0+0x00000000c9c7)
30:     #14 platform_thread_wrap /home/owend/master/platform/src/cb_pthreads.cc:71 (libplatform_so.so.0.1.0+0x00000000affe)
30:     #15 <null> <null> (libtsan.so.0+0x0000000230d9)
30:
30:   Previous write of size 8 at 0x7d1800060a38 by main thread (mutexes: write M1575):
30:     #0 operator new(unsigned long) <null> (libtsan.so.0+0x000000025a33)
30:     #1 __gnu_cxx::new_allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::allocate(unsigned long, void const*) <null> (engine_testapp+0x0000004a6ca7)
30:     #2 std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > >::allocate(std::allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&, unsigned long) <null> (engine_testapp+0x0000004a5168)
30:     #3 std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::_M_get_node() <null> (engine_testapp+0x0000004a316f)
30:     #4 std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >* std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::_M_create_node<std::piecewise_construct_t const&, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>, std::tuple<> >(std::piecewise_construct_t const&, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>&&, std::tuple<>&&) <null> (engine_testapp+0x00000049c6b4)
30:     #5 std::_Rb_tree_iterator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::_M_emplace_hint_unique<std::piecewise_construct_t const&, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>, std::tuple<> >(std::_Rb_tree_const_iterator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::piecewise_construct_t const&, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>&&, std::tuple<>&&) <null> (engine_testapp+0x00000049928e)
30:     #6 std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::operator[](std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) <null> (engine_testapp+0x00000048e488)
30:     #7 add_stats /home/owend/master/ep-engine/tests/ep_test_apis.cc:236 (ep_perfsuite.so+0x00000008f5b9)
30:     #8 STATWRITER_NAMESPACE::add_casted_stat(char const*, char const*, void (*)(char const*, unsigned short, char const*, unsigned int, void const*), void const*) /home/owend/master/ep-engine/src/statwriter.h:39 (ep.so+0x0000000e5673)
30:     #9 void STATWRITER_NAMESPACE::add_casted_stat<unsigned long>(char const*, unsigned long const&, void (*)(char const*, unsigned short, char const*, unsigned int, void const*), void const*) /home/owend/master/ep-engine/src/statwriter.h:48 (ep.so+0x0000000e96ae)
30:     #10 EventuallyPersistentEngine::addSeqnoVbStats(void const*, void (*)(char const*, unsigned short, char const*, unsigned int, void const*), RCPtr<VBucket> const&) /home/owend/master/ep-engine/src/ep_engine.cc:4445 (ep.so+0x0000001ad56d)
30:     #11 EventuallyPersistentEngine::doSeqnoStats(void const*, void (*)(char const*, unsigned short, char const*, unsigned int, void const*), char const*, int) /home/owend/master/ep-engine/src/ep_engine.cc:4490 (ep.so+0x0000001ad95f)
30:     #12 EventuallyPersistentEngine::getStats(void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)) /home/owend/master/ep-engine/src/ep_engine.cc:4595 (ep.so+0x0000001ae4de)
30:     #13 EvpGetStats /home/owend/master/ep-engine/src/ep_engine.cc:232 (ep.so+0x00000019851e)
30:     #14 ENGINE_ERROR_CODE std::_Bind<ENGINE_ERROR_CODE (*(engine_interface*, void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)))(engine_interface*, void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*))>::__call<ENGINE_ERROR_CODE, , 0ul, 1ul, 2ul, 3ul, 4ul>(std::tuple<>&&, std::_Index_tuple<0ul, 1ul, 2ul, 3ul, 4ul>) <null> (engine_testapp+0x00000049f211)
30:     #15 ENGINE_ERROR_CODE std::_Bind<ENGINE_ERROR_CODE (*(engine_interface*, void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)))(engine_interface*, void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*))>::operator()<, ENGINE_ERROR_CODE>() <null> (engine_testapp+0x00000049aa02)
30:     #16 std::_Function_handler<ENGINE_ERROR_CODE (), std::_Bind<ENGINE_ERROR_CODE (*(engine_interface*, void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)))(engine_interface*, void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*))> >::_M_invoke(std::_Any_data const&) <null> (engine_testapp+0x00000049145b)
30:     #17 std::function<ENGINE_ERROR_CODE ()>::operator()() const <null> (engine_testapp+0x000000484e28)
30:     #18 call_engine_and_handle_EWOULDBLOCK /home/owend/master/memcached/programs/engine_testapp/engine_testapp.cc:127 (engine_testapp+0x00000047cf00)
30:     #19 mock_get_stats /home/owend/master/memcached/programs/engine_testapp/engine_testapp.cc:220 (engine_testapp+0x00000047d896)
30:     #20 perf_stat_latency_core /home/owend/master/ep-engine/tests/ep_perfsuite.cc:1238 (ep_perfsuite.so+0x000000063d3d)
30:     #21 perf_stat_latency /home/owend/master/ep-engine/tests/ep_perfsuite.cc:1306 (ep_perfsuite.so+0x00000006427a)
30:     #22 perf_slow_stat_latency_100vb /home/owend/master/ep-engine/tests/ep_perfsuite.cc:1365 (ep_perfsuite.so+0x00000006498c)
30:     #23 execute_test /home/owend/master/memcached/programs/engine_testapp/engine_testapp.cc:962 (engine_testapp+0x000000481776)
30:     #24 main /home/owend/master/memcached/programs/engine_testapp/engine_testapp.cc:1359 (engine_testapp+0x000000482ab1)

Change-Id: I7bdc847c0913244409fa044e312d53b484dc2dab
Reviewed-on: http://review.couchbase.org/68813
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: buildbot <build@couchbase.com>

Author: owend74

tests/ep_test_apis.cc

@@ -33,18 +33,31 @@
 
 #include "mock/mock_dcp.h"
 
+template<typename T> class HistogramStats;
+
 // Due to the limitations of the add_stats callback (essentially we cannot pass
-// a context into it) we instead have a single, global `vals` map. This mutex
-// is used to serialise modifications to it to allow multiple threads to request
-// stats.
-// There is also an optimized add_stats callback (add_individual_stat) which
-// checks for one stat (and hence doesn't have to keep a map of all of them).
-// the vals_mutex is also used for serializing acccess to it's data
-// (requested_stat_name and actual_stat_value).
+// a context into it) we instead have a single, global `vals` map. The
+// vals_mutex is to ensure serialised modifications to this data structure.
 std::mutex vals_mutex;
 statistic_map vals;
-std::string requested_stat_name;
-std::string actual_stat_value;
+
+// get_stat and get_histo_stat can only be called one at a time as they use
+// the three global variables (requested_stat_name, actual_stat_value and
+// histogram_stat_int_value).  Therefore the two functions need to acquire a
+// lock and keep it for the whole function duration.
+
+// The requested_stat_name and actual_stat_value are used in an optimized
+// add_stats callback (add_individual_stat) which checks for one stat
+// (and hence doesn't have to keep a map of all of them).
+struct {
+    std::mutex mutex;
+    std::string requested_stat_name;
+    std::string actual_stat_value;
+    /* HistogramStats<T>* is supported C++14 onwards.
+     * Until then use a separate ptr for each type.
+     */
+    HistogramStats<int>* histogram_stat_int_value;
+} get_stat_context;
 
 bool dump_stats = false;
 std::atomic<protocol_binary_response_status> last_status(
@@ -118,10 +131,6 @@ class HistogramStats {
     uint64_t total_count;
 };
 
-/* HistogramStats<T>* is supported C++14 onwards. Until then use a separate
-   ptr for each type */
-static HistogramStats<int>* histogram_stat_int_value;
-
 static void get_histo_stat(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
                                   const char *statname, const char *statkey);
 
@@ -233,6 +242,7 @@ void add_stats(const char *key, const uint16_t klen, const char *val,
         std::cout << "stat[" << k << "] = " << v << std::endl;
     }
 
+    std::lock_guard<std::mutex> lh(vals_mutex);
     vals[k] = v;
 }
 
@@ -243,10 +253,11 @@ void add_stats(const char *key, const uint16_t klen, const char *val,
 void add_individual_stat(const char *key, const uint16_t klen, const char *val,
                const uint32_t vlen, const void *cookie) {
 
-    if (actual_stat_value.empty() &&
-        requested_stat_name.compare(0, requested_stat_name.size(),
-                                    key, klen) == 0) {
-        actual_stat_value = std::string(val, vlen);
+    if (get_stat_context.actual_stat_value.empty() &&
+            get_stat_context.requested_stat_name.compare(
+                    0, get_stat_context.requested_stat_name.size(),
+                    key, klen) == 0) {
+        get_stat_context.actual_stat_value = std::string(val, vlen);
     }
 }
 
@@ -255,13 +266,13 @@ void add_individual_histo_stat(const char *key, const uint16_t klen,
                                const void *cookie) {
     /* Convert key to string */
     std::string key_str(key, klen);
-    size_t pos1 = key_str.find(requested_stat_name);
+    size_t pos1 = key_str.find(get_stat_context.requested_stat_name);
     if (pos1 != std::string::npos)
     {
-        actual_stat_value.append(val, vlen);
+        get_stat_context.actual_stat_value.append(val, vlen);
         /* Parse start and end from the key.
            Key is in the format task_name_START,END (backfill_tasks_20,100) */
-        pos1 += requested_stat_name.length();
+        pos1 += get_stat_context.requested_stat_name.length();
         /* Find ',' to move to end of bin_start */
         size_t pos2 = key_str.find(',', pos1);
         if ((std::string::npos == pos2) || (pos1 >= pos2)) {
@@ -277,7 +288,8 @@ void add_individual_histo_stat(const char *key, const uint16_t klen,
             throw std::invalid_argument("Malformed histogram stat: " + key_str);
         }
         int end = std::stoi(std::string(key_str, pos1, pos2));
-        histogram_stat_int_value->add_bin(start, end, std::stoull(val));
+        get_stat_context.histogram_stat_int_value->add_bin(start, end,
+                                                           std::stoull(val));
     }
 }
 
@@ -1049,17 +1061,23 @@ bool verify_vbucket_missing(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
 
     // Try up to three times to verify the bucket is missing.  Bucket
     // state changes are async.
-    std::lock_guard<std::mutex> lh(vals_mutex);
-    vals.clear();
+    {
+        std::lock_guard<std::mutex> lh(vals_mutex);
+        vals.clear();
+    }
+
     check(h1->get_stats(h, NULL, NULL, 0, add_stats) == ENGINE_SUCCESS,
           "Failed to get stats.");
 
-    if (vals.find(vbstr) == vals.end()) {
-        return true;
-    }
-
-    std::cerr << "Expected bucket missing, got " << vals[vbstr] << std::endl;
+    {
+        std::lock_guard<std::mutex> lh(vals_mutex);
+        if (vals.find(vbstr) == vals.end()) {
+            return true;
+        }
 
+        std::cerr << "Expected bucket missing, got " <<
+                vals[vbstr] << std::endl;
+    }
     return false;
 }
 
@@ -1143,10 +1161,10 @@ bool get_stat(ENGINE_HANDLE* h, ENGINE_HANDLE_V1* h1,
 template<>
 std::string get_stat(ENGINE_HANDLE* h, ENGINE_HANDLE_V1* h1,
                      const char* statname, const char* statkey) {
-    std::lock_guard<std::mutex> lh(vals_mutex);
+    std::lock_guard<std::mutex> lh(get_stat_context.mutex);
 
-    requested_stat_name = statname;
-    actual_stat_value.clear();
+    get_stat_context.requested_stat_name = statname;
+    get_stat_context.actual_stat_value.clear();
 
     ENGINE_ERROR_CODE err = h1->get_stats(h, NULL, statkey,
                                           statkey == NULL ? 0 : strlen(statkey),
@@ -1156,15 +1174,16 @@ std::string get_stat(ENGINE_HANDLE* h, ENGINE_HANDLE_V1* h1,
         throw engine_error(err);
     }
 
-    if (actual_stat_value.empty()) {
+    if (get_stat_context.actual_stat_value.empty()) {
         throw std::out_of_range(std::string("Failed to find requested statname '") +
                                 statname + "'");
     }
 
     // Here we are explictly forcing a copy of the object to work
     // around std::string copy-on-write data-race issues seen on some
     // versions of libstdc++ - see MB-18510 / MB-19688.
-    return std::string(actual_stat_value.begin(), actual_stat_value.end());
+    return std::string(get_stat_context.actual_stat_value.begin(),
+                       get_stat_context.actual_stat_value.end());
 }
 
 /// Backward-compatible functions (encode type name in function name).
@@ -1224,34 +1243,35 @@ uint64_t get_histo_stat(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
                         const char *statname, const char *statkey,
                         const Histo_stat_info histo_info)
 {
-    std::lock_guard<std::mutex> lh(vals_mutex);
+    std::lock_guard<std::mutex> lh(get_stat_context.mutex);
 
-    histogram_stat_int_value = new HistogramStats<int>();
+    get_stat_context.histogram_stat_int_value = new HistogramStats<int>();
     get_histo_stat(h, h1, statname, statkey);
 
     /* Get the necessary info from the histogram */
     uint64_t ret_val = 0;
     switch (histo_info) {
         case Histo_stat_info::TOTAL_COUNT:
-            ret_val = histogram_stat_int_value->total();
+            ret_val = get_stat_context.histogram_stat_int_value->total();
             break;
         case Histo_stat_info::NUM_BINS:
             ret_val =
-                    static_cast<uint64_t>(histogram_stat_int_value->num_bins());
+                    static_cast<uint64_t>(get_stat_context.
+                                          histogram_stat_int_value->num_bins());
             break;
     }
 
-    delete histogram_stat_int_value;
+    delete get_stat_context.histogram_stat_int_value;
     return ret_val;
 }
 
 static void get_histo_stat(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
                                   const char *statname, const char *statkey)
 {
-    requested_stat_name = statname;
+    get_stat_context.requested_stat_name = statname;
     /* Histo stats for tasks are append as task_name_START,END.
        Hence append _ */
-    requested_stat_name.append("_");
+    get_stat_context.requested_stat_name.append("_");
 
     ENGINE_ERROR_CODE err = h1->get_stats(h, NULL, statkey,
                                           statkey == NULL ? 0 : strlen(statkey),
@@ -1266,9 +1286,10 @@ static void get_histo_stat(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
 
 statistic_map get_all_stats(ENGINE_HANDLE *h,ENGINE_HANDLE_V1 *h1,
                             const char *statset) {
-    std::lock_guard<std::mutex> lh(vals_mutex);
-
-    vals.clear();
+    {
+        std::lock_guard<std::mutex> lh(vals_mutex);
+        vals.clear();
+    }
     ENGINE_ERROR_CODE err = h1->get_stats(h, NULL, statset,
                                           statset == NULL ? 0 : strlen(statset),
                                           add_stats);
@@ -1277,6 +1298,7 @@ statistic_map get_all_stats(ENGINE_HANDLE *h,ENGINE_HANDLE_V1 *h1,
         throw engine_error(err);
     }
 
+    std::lock_guard<std::mutex> lh(vals_mutex);
     return vals;
 }