Skip to content

Commit ca75ef6

Browse files
author
lexeyo
committed
fix engine: fix UB in deadlock detector on diamond-shaped dependencies
Fixes incorrect handling of diamond-shaped dependencies in CycleDetector. commit_hash:bf7e71dbe49cee9ec613214e1dcc540b80204f97
1 parent 9720edf commit ca75ef6

File tree

2 files changed

+46
-10
lines changed

2 files changed

+46
-10
lines changed

core/src/engine/deadlock_detector.cpp

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,31 +57,33 @@ class CycleDetector final {
5757
std::vector<Vertex> cycle;
5858
};
5959

60-
void HandleVertex(Vertex v, Vertex parent) {
61-
auto it_p = parents_.find(v);
62-
if (it_p != parents_.end()) {
60+
void HandleVertex(Vertex v, Vertex previous) {
61+
auto it_p = vertex_to_previous_vertex_.find(v);
62+
if (it_p != vertex_to_previous_vertex_.end()) {
6363
auto finish = it_p->first;
6464

6565
std::vector<Vertex> cycle;
6666
cycle.push_back(finish);
67-
while (parent != finish) {
68-
cycle.push_back(parent);
69-
parent = parents_[parent];
67+
while (previous != finish) {
68+
cycle.push_back(previous);
69+
previous = vertex_to_previous_vertex_[previous];
7070
}
7171
throw CycleException(std::move(cycle));
7272
}
7373

7474
auto it = edges_.find(v);
75-
if (it != edges_.end()) {
75+
if (it != edges_.end() && processed_.find(v) == processed_.end()) {
7676
dfs_processing_stack_.emplace_back(DfsStackItem{v, it->second.begin(), it->second.end()});
77-
parents_[v] = parent;
77+
vertex_to_previous_vertex_[v] = previous;
7878
}
7979
}
8080

8181
void DoFindCycle() {
8282
while (!dfs_processing_stack_.empty()) {
8383
auto& frame = dfs_processing_stack_.back();
8484
if (frame.iter == frame.end) {
85+
vertex_to_previous_vertex_.erase(frame.v);
86+
processed_.emplace(frame.v);
8587
dfs_processing_stack_.pop_back();
8688
continue;
8789
}
@@ -95,8 +97,10 @@ class CycleDetector final {
9597

9698
std::vector<DfsStackItem> dfs_processing_stack_;
9799

98-
// child -> parent
99-
std::unordered_map<Vertex, Vertex> parents_;
100+
// Allows both visited vertex tracking and backward path reconstruction
101+
std::unordered_map<Vertex, Vertex> vertex_to_previous_vertex_;
102+
103+
std::unordered_set<Vertex> processed_;
100104
};
101105

102106
} // namespace

core/src/engine/deadlock_detector_test.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,4 +106,36 @@ UTEST(DeadlockDetector, RightCycle) {
106106
EXPECT_THROW_CYCLE(state.HookBeforeAddDependency(a2, a1), &a1, &a2);
107107
}
108108

109+
UTEST(DeadlockDetector, NoCycleOnDiamond) {
110+
/* Tests processing of a diamond shape:
111+
// start
112+
// |
113+
// 0
114+
// / \
115+
// / \
116+
// 1 4
117+
// \ /
118+
// \ /
119+
// 2
120+
// |
121+
// 3
122+
*/
123+
MockState state;
124+
MockActor start;
125+
MockActor v0;
126+
MockActor v1;
127+
MockActor v2;
128+
MockActor v3;
129+
MockActor v4;
130+
131+
state.HookBeforeAddDependency(v0, v1);
132+
state.HookBeforeAddDependency(v0, v4);
133+
state.HookBeforeAddDependency(v1, v2);
134+
state.HookBeforeAddDependency(v4, v2);
135+
state.HookBeforeAddDependency(v2, v3);
136+
state.HookBeforeAddDependency(start, v0);
137+
138+
SUCCEED();
139+
}
140+
109141
USERVER_NAMESPACE_END

0 commit comments

Comments
 (0)