From fedaa12737d4c347b00275f3f4d323e58b4c6689 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 8 Jan 2021 15:55:49 +0800 Subject: [PATCH] common/Throttle: return new count in put() right away before this change, the is a racing after updating `count`, where `count` could be updated again by another thread using this Throttle instance. but the caller expects the value before the second mutation. actually, the only caller which care about the return value is the unittests in test/common/Throttle.cc. and without this change, the ThrottleTest.get test fails randomly. almost 1 out of 5 runs fails. after this change, the test passes after running for 256 times without failures. Signed-off-by: Kefu Chai --- src/common/Throttle.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/common/Throttle.cc b/src/common/Throttle.cc index b8b4783dce3..2e5b3459c39 100644 --- a/src/common/Throttle.cc +++ b/src/common/Throttle.cc @@ -228,14 +228,16 @@ int64_t Throttle::put(int64_t c) ceph_assert(c >= 0); ldout(cct, 10) << "put " << c << " (" << count.load() << " -> " << (count.load()-c) << ")" << dendl; + int64_t new_count; { std::lock_guard l(lock); + new_count = count; if (c) { if (!conds.empty()) conds.front().notify_one(); // if count goes negative, we failed somewhere! ceph_assert(count >= c); - count -= c; + new_count = count -= c; } } if (logger) { @@ -244,7 +246,7 @@ int64_t Throttle::put(int64_t c) logger->set(l_throttle_val, count); } - return count; + return new_count; } void Throttle::reset() -- 2.39.5