Skip to content

Commit d775e16

Browse files
owend74daverigby
owend74
authored and
daverigby
committed
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>
1 parent f4e92d8 commit d775e16

1 file changed

Lines changed: 65 additions & 43 deletions

File tree

‎tests/ep_test_apis.cc‎

Lines changed: 65 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,31 @@
3333

3434
#include "mock/mock_dcp.h"
3535

36+
template<typename T> class HistogramStats;
37+
3638
// Due to the limitations of the add_stats callback (essentially we cannot pass
37-
// a context into it) we instead have a single, global `vals` map. This mutex
38-
// is used to serialise modifications to it to allow multiple threads to request
39-
// stats.
40-
// There is also an optimized add_stats callback (add_individual_stat) which
41-
// checks for one stat (and hence doesn't have to keep a map of all of them).
42-
// the vals_mutex is also used for serializing acccess to it's data
43-
// (requested_stat_name and actual_stat_value).
39+
// a context into it) we instead have a single, global `vals` map. The
40+
// vals_mutex is to ensure serialised modifications to this data structure.
4441
std::mutex vals_mutex;
4542
statistic_map vals;
46-
std::string requested_stat_name;
47-
std::string actual_stat_value;
43+
44+
// get_stat and get_histo_stat can only be called one at a time as they use
45+
// the three global variables (requested_stat_name, actual_stat_value and
46+
// histogram_stat_int_value). Therefore the two functions need to acquire a
47+
// lock and keep it for the whole function duration.
48+
49+
// The requested_stat_name and actual_stat_value are used in an optimized
50+
// add_stats callback (add_individual_stat) which checks for one stat
51+
// (and hence doesn't have to keep a map of all of them).
52+
struct {
53+
std::mutex mutex;
54+
std::string requested_stat_name;
55+
std::string actual_stat_value;
56+
/* HistogramStats<T>* is supported C++14 onwards.
57+
* Until then use a separate ptr for each type.
58+
*/
59+
HistogramStats<int>* histogram_stat_int_value;
60+
} get_stat_context;
4861

4962
bool dump_stats = false;
5063
std::atomic<protocol_binary_response_status> last_status(
@@ -118,10 +131,6 @@ class HistogramStats {
118131
uint64_t total_count;
119132
};
120133

121-
/* HistogramStats<T>* is supported C++14 onwards. Until then use a separate
122-
ptr for each type */
123-
static HistogramStats<int>* histogram_stat_int_value;
124-
125134
static void get_histo_stat(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
126135
const char *statname, const char *statkey);
127136

@@ -233,6 +242,7 @@ void add_stats(const char *key, const uint16_t klen, const char *val,
233242
std::cout << "stat[" << k << "] = " << v << std::endl;
234243
}
235244

245+
std::lock_guard<std::mutex> lh(vals_mutex);
236246
vals[k] = v;
237247
}
238248

@@ -243,10 +253,11 @@ void add_stats(const char *key, const uint16_t klen, const char *val,
243253
void add_individual_stat(const char *key, const uint16_t klen, const char *val,
244254
const uint32_t vlen, const void *cookie) {
245255

246-
if (actual_stat_value.empty() &&
247-
requested_stat_name.compare(0, requested_stat_name.size(),
248-
key, klen) == 0) {
249-
actual_stat_value = std::string(val, vlen);
256+
if (get_stat_context.actual_stat_value.empty() &&
257+
get_stat_context.requested_stat_name.compare(
258+
0, get_stat_context.requested_stat_name.size(),
259+
key, klen) == 0) {
260+
get_stat_context.actual_stat_value = std::string(val, vlen);
250261
}
251262
}
252263

@@ -255,13 +266,13 @@ void add_individual_histo_stat(const char *key, const uint16_t klen,
255266
const void *cookie) {
256267
/* Convert key to string */
257268
std::string key_str(key, klen);
258-
size_t pos1 = key_str.find(requested_stat_name);
269+
size_t pos1 = key_str.find(get_stat_context.requested_stat_name);
259270
if (pos1 != std::string::npos)
260271
{
261-
actual_stat_value.append(val, vlen);
272+
get_stat_context.actual_stat_value.append(val, vlen);
262273
/* Parse start and end from the key.
263274
Key is in the format task_name_START,END (backfill_tasks_20,100) */
264-
pos1 += requested_stat_name.length();
275+
pos1 += get_stat_context.requested_stat_name.length();
265276
/* Find ',' to move to end of bin_start */
266277
size_t pos2 = key_str.find(',', pos1);
267278
if ((std::string::npos == pos2) || (pos1 >= pos2)) {
@@ -277,7 +288,8 @@ void add_individual_histo_stat(const char *key, const uint16_t klen,
277288
throw std::invalid_argument("Malformed histogram stat: " + key_str);
278289
}
279290
int end = std::stoi(std::string(key_str, pos1, pos2));
280-
histogram_stat_int_value->add_bin(start, end, std::stoull(val));
291+
get_stat_context.histogram_stat_int_value->add_bin(start, end,
292+
std::stoull(val));
281293
}
282294
}
283295

@@ -1049,17 +1061,23 @@ bool verify_vbucket_missing(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
10491061

10501062
// Try up to three times to verify the bucket is missing. Bucket
10511063
// state changes are async.
1052-
std::lock_guard<std::mutex> lh(vals_mutex);
1053-
vals.clear();
1064+
{
1065+
std::lock_guard<std::mutex> lh(vals_mutex);
1066+
vals.clear();
1067+
}
1068+
10541069
check(h1->get_stats(h, NULL, NULL, 0, add_stats) == ENGINE_SUCCESS,
10551070
"Failed to get stats.");
10561071

1057-
if (vals.find(vbstr) == vals.end()) {
1058-
return true;
1059-
}
1060-
1061-
std::cerr << "Expected bucket missing, got " << vals[vbstr] << std::endl;
1072+
{
1073+
std::lock_guard<std::mutex> lh(vals_mutex);
1074+
if (vals.find(vbstr) == vals.end()) {
1075+
return true;
1076+
}
10621077

1078+
std::cerr << "Expected bucket missing, got " <<
1079+
vals[vbstr] << std::endl;
1080+
}
10631081
return false;
10641082
}
10651083

@@ -1143,10 +1161,10 @@ bool get_stat(ENGINE_HANDLE* h, ENGINE_HANDLE_V1* h1,
11431161
template<>
11441162
std::string get_stat(ENGINE_HANDLE* h, ENGINE_HANDLE_V1* h1,
11451163
const char* statname, const char* statkey) {
1146-
std::lock_guard<std::mutex> lh(vals_mutex);
1164+
std::lock_guard<std::mutex> lh(get_stat_context.mutex);
11471165

1148-
requested_stat_name = statname;
1149-
actual_stat_value.clear();
1166+
get_stat_context.requested_stat_name = statname;
1167+
get_stat_context.actual_stat_value.clear();
11501168

11511169
ENGINE_ERROR_CODE err = h1->get_stats(h, NULL, statkey,
11521170
statkey == NULL ? 0 : strlen(statkey),
@@ -1156,15 +1174,16 @@ std::string get_stat(ENGINE_HANDLE* h, ENGINE_HANDLE_V1* h1,
11561174
throw engine_error(err);
11571175
}
11581176

1159-
if (actual_stat_value.empty()) {
1177+
if (get_stat_context.actual_stat_value.empty()) {
11601178
throw std::out_of_range(std::string("Failed to find requested statname '") +
11611179
statname + "'");
11621180
}
11631181

11641182
// Here we are explictly forcing a copy of the object to work
11651183
// around std::string copy-on-write data-race issues seen on some
11661184
// versions of libstdc++ - see MB-18510 / MB-19688.
1167-
return std::string(actual_stat_value.begin(), actual_stat_value.end());
1185+
return std::string(get_stat_context.actual_stat_value.begin(),
1186+
get_stat_context.actual_stat_value.end());
11681187
}
11691188

11701189
/// 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,
12241243
const char *statname, const char *statkey,
12251244
const Histo_stat_info histo_info)
12261245
{
1227-
std::lock_guard<std::mutex> lh(vals_mutex);
1246+
std::lock_guard<std::mutex> lh(get_stat_context.mutex);
12281247

1229-
histogram_stat_int_value = new HistogramStats<int>();
1248+
get_stat_context.histogram_stat_int_value = new HistogramStats<int>();
12301249
get_histo_stat(h, h1, statname, statkey);
12311250

12321251
/* Get the necessary info from the histogram */
12331252
uint64_t ret_val = 0;
12341253
switch (histo_info) {
12351254
case Histo_stat_info::TOTAL_COUNT:
1236-
ret_val = histogram_stat_int_value->total();
1255+
ret_val = get_stat_context.histogram_stat_int_value->total();
12371256
break;
12381257
case Histo_stat_info::NUM_BINS:
12391258
ret_val =
1240-
static_cast<uint64_t>(histogram_stat_int_value->num_bins());
1259+
static_cast<uint64_t>(get_stat_context.
1260+
histogram_stat_int_value->num_bins());
12411261
break;
12421262
}
12431263

1244-
delete histogram_stat_int_value;
1264+
delete get_stat_context.histogram_stat_int_value;
12451265
return ret_val;
12461266
}
12471267

12481268
static void get_histo_stat(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
12491269
const char *statname, const char *statkey)
12501270
{
1251-
requested_stat_name = statname;
1271+
get_stat_context.requested_stat_name = statname;
12521272
/* Histo stats for tasks are append as task_name_START,END.
12531273
Hence append _ */
1254-
requested_stat_name.append("_");
1274+
get_stat_context.requested_stat_name.append("_");
12551275

12561276
ENGINE_ERROR_CODE err = h1->get_stats(h, NULL, statkey,
12571277
statkey == NULL ? 0 : strlen(statkey),
@@ -1266,9 +1286,10 @@ static void get_histo_stat(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
12661286

12671287
statistic_map get_all_stats(ENGINE_HANDLE *h,ENGINE_HANDLE_V1 *h1,
12681288
const char *statset) {
1269-
std::lock_guard<std::mutex> lh(vals_mutex);
1270-
1271-
vals.clear();
1289+
{
1290+
std::lock_guard<std::mutex> lh(vals_mutex);
1291+
vals.clear();
1292+
}
12721293
ENGINE_ERROR_CODE err = h1->get_stats(h, NULL, statset,
12731294
statset == NULL ? 0 : strlen(statset),
12741295
add_stats);
@@ -1277,6 +1298,7 @@ statistic_map get_all_stats(ENGINE_HANDLE *h,ENGINE_HANDLE_V1 *h1,
12771298
throw engine_error(err);
12781299
}
12791300

1301+
std::lock_guard<std::mutex> lh(vals_mutex);
12801302
return vals;
12811303
}
12821304

0 commit comments

Comments
 (0)