github-actions[bot] commented on code in PR #65620:
URL: https://github.com/apache/doris/pull/65620#discussion_r3594303793
##########
be/src/util/debug_points.cpp:
##########
@@ -44,10 +54,16 @@ std::shared_ptr<DebugPoint>
DebugPoints::get_debug_point(const std::string& name
}
auto debug_point = it->second;
- if ((debug_point->expire_ms > 0 && MonotonicMillis() >=
debug_point->expire_ms) ||
- (debug_point->execute_limit > 0 &&
- debug_point->execute_num.fetch_add(1, std::memory_order_relaxed) >=
- debug_point->execute_limit)) {
+ if (debug_point->expire_ms > 0 && MonotonicMillis() >=
debug_point->expire_ms) {
+ remove(name);
+ return nullptr;
+ }
+ if (predicate != nullptr && !(*predicate)(*debug_point)) {
+ return nullptr;
+ }
+ if (debug_point->execute_limit > 0 &&
+ debug_point->execute_num.fetch_add(1, std::memory_order_relaxed) >=
+ debug_point->execute_limit) {
remove(name);
Review Comment:
`get_debug_point_impl()` can erase a newer same-name point here. T1 can load
exhausted point A and pause in the new caller predicate; T2 can re-add the name
with point B; when T1 resumes, this name-only `remove(name)` deletes B from the
latest map. The old lookup had a much narrower version of this race, but the
new arbitrary predicate interval makes it deterministic and same-name
replacement is used by fault-injection cases. Please erase only when the
current map entry is still the evaluated `shared_ptr`, and add a barrier-based
unit test showing B survives replacement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
