Skip to content

Commit 02377be

Browse files
author
lexeyo
committed
refactor core: deadlock detector refactoring
* Moves actor.hpp to includes. * Changes Actor's .dtor to non-virtual. commit_hash:85a110438fe54185f18430cf251a74361e91388a
1 parent c50c869 commit 02377be

File tree

9 files changed

+35
-33
lines changed

9 files changed

+35
-33
lines changed

‎.mapping.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,7 @@
959959
"core/include/userver/engine/future.hpp":"taxi/uservices/userver/core/include/userver/engine/future.hpp",
960960
"core/include/userver/engine/future_status.hpp":"taxi/uservices/userver/core/include/userver/engine/future_status.hpp",
961961
"core/include/userver/engine/get_all.hpp":"taxi/uservices/userver/core/include/userver/engine/get_all.hpp",
962+
"core/include/userver/engine/impl/actor.hpp":"taxi/uservices/userver/core/include/userver/engine/impl/actor.hpp",
962963
"core/include/userver/engine/impl/condition_variable_any.hpp":"taxi/uservices/userver/core/include/userver/engine/impl/condition_variable_any.hpp",
963964
"core/include/userver/engine/impl/context_accessor.hpp":"taxi/uservices/userver/core/include/userver/engine/impl/context_accessor.hpp",
964965
"core/include/userver/engine/impl/detached_tasks_sync_block.hpp":"taxi/uservices/userver/core/include/userver/engine/impl/detached_tasks_sync_block.hpp",
@@ -1505,7 +1506,6 @@
15051506
"core/src/engine/deadline_test.cpp":"taxi/uservices/userver/core/src/engine/deadline_test.cpp",
15061507
"core/src/engine/deadlock_detector.cpp":"taxi/uservices/userver/core/src/engine/deadlock_detector.cpp",
15071508
"core/src/engine/deadlock_detector.hpp":"taxi/uservices/userver/core/src/engine/deadlock_detector.hpp",
1508-
"core/src/engine/deadlock_detector/actor.hpp":"taxi/uservices/userver/core/src/engine/deadlock_detector/actor.hpp",
15091509
"core/src/engine/deadlock_detector_config.cpp":"taxi/uservices/userver/core/src/engine/deadlock_detector_config.cpp",
15101510
"core/src/engine/deadlock_detector_test.cpp":"taxi/uservices/userver/core/src/engine/deadlock_detector_test.cpp",
15111511
"core/src/engine/errno_test.cpp":"taxi/uservices/userver/core/src/engine/errno_test.cpp",

core/src/engine/deadlock_detector/actor.hpp renamed to core/include/userver/engine/impl/actor.hpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,20 @@
66

77
USERVER_NAMESPACE_BEGIN
88

9-
namespace engine::deadlock_detector {
9+
namespace engine::impl::deadlock_detector {
1010

1111
class Actor {
1212
public:
13-
virtual ~Actor() = default;
14-
1513
virtual utils::StringLiteral GetActorType() const = 0;
14+
15+
protected:
16+
~Actor() = default;
1617
};
1718

1819
inline std::string ToAssertString(const Actor& a) {
1920
return fmt::format("{} (ptr={})", a.GetActorType(), static_cast<const void*>(&a));
2021
}
2122

22-
} // namespace engine::deadlock_detector
23+
} // namespace engine::impl::deadlock_detector
2324

2425
USERVER_NAMESPACE_END

‎core/src/engine/deadlock_detector.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ namespace {
2424

2525
class CycleDetector final {
2626
public:
27+
using Actor = engine::impl::deadlock_detector::Actor;
2728
using Vertex = const Actor*;
2829

2930
explicit CycleDetector(const std::unordered_map<const Actor*, std::vector<const Actor*>>& edges)
@@ -232,7 +233,7 @@ State& GetState() {
232233
return pool.GetDeadlockDetectorState();
233234
}
234235

235-
WaitScope::WaitScope(const Actor& resource)
236+
WaitScope::WaitScope(const engine::impl::deadlock_detector::Actor& resource)
236237
: resource_(resource)
237238
{
238239
auto& dd_state = GetState();

‎core/src/engine/deadlock_detector.hpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
#include <engine/task/task_context.hpp>
66

77
#include <engine/coro/pool_config.hpp>
8-
#include <engine/deadlock_detector/actor.hpp>
8+
#include <userver/engine/impl/actor.hpp>
99
#include <userver/utils/fast_pimpl.hpp>
1010

1111
USERVER_NAMESPACE_BEGIN
@@ -14,6 +14,8 @@ namespace engine::deadlock_detector {
1414

1515
class StateBase {
1616
public:
17+
using Actor = engine::impl::deadlock_detector::Actor;
18+
1719
explicit StateBase(DeadlockDetector dd);
1820

1921
virtual ~StateBase();
@@ -44,7 +46,7 @@ class State final : public StateBase {
4446
protected:
4547
using StateBase::StateBase;
4648

47-
void OnCycleFound(const std::vector<const Actor*>& cycle) override;
49+
void OnCycleFound(const std::vector<const StateBase::Actor*>& cycle) override;
4850
};
4951

5052
// The state is global to the process
@@ -53,12 +55,12 @@ State& GetState();
5355

5456
class WaitScope final {
5557
public:
56-
explicit WaitScope(const Actor& resource);
58+
explicit WaitScope(const engine::impl::deadlock_detector::Actor& resource);
5759
WaitScope(WaitScope&&) = delete;
5860
~WaitScope();
5961

6062
private:
61-
const Actor& resource_;
63+
const engine::impl::deadlock_detector::Actor& resource_;
6264
};
6365

6466
} // namespace engine::deadlock_detector

‎core/src/engine/deadlock_detector_test.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,21 @@ TEST(DeadlockDetectorDeathTest, Smoke) {
2727
}
2828

2929
struct CycleDetected : public std::runtime_error {
30-
CycleDetected(std::vector<const engine::deadlock_detector::Actor*> cycle)
30+
CycleDetected(std::vector<const engine::impl::deadlock_detector::Actor*> cycle)
3131
: std::runtime_error(""),
3232
cycle(std::move(cycle))
3333
{}
3434

35-
std::vector<const engine::deadlock_detector::Actor*> cycle;
35+
std::vector<const engine::impl::deadlock_detector::Actor*> cycle;
3636
};
3737

3838
// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
39-
#define EXPECT_THROW_CYCLE(cmd, ...) \
40-
try { \
41-
cmd; \
42-
FAIL() << "Deadlock was expected, but not found"; \
43-
} catch (const CycleDetected& c) { \
44-
EXPECT_EQ(c.cycle, (std::vector<const engine::deadlock_detector::Actor*>{__VA_ARGS__})); \
39+
#define EXPECT_THROW_CYCLE(cmd, ...) \
40+
try { \
41+
cmd; \
42+
FAIL() << "Deadlock was expected, but not found"; \
43+
} catch (const CycleDetected& c) { \
44+
EXPECT_EQ(c.cycle, (std::vector<const engine::impl::deadlock_detector::Actor*>{__VA_ARGS__})); \
4545
}
4646

4747
class MockState final : public engine::deadlock_detector::StateBase {
@@ -51,12 +51,12 @@ class MockState final : public engine::deadlock_detector::StateBase {
5151
{}
5252

5353
protected:
54-
void OnCycleFound(const std::vector<const engine::deadlock_detector::Actor*>& cycle) override {
54+
void OnCycleFound(const std::vector<const engine::impl::deadlock_detector::Actor*>& cycle) override {
5555
throw CycleDetected(cycle);
5656
}
5757
};
5858

59-
class MockActor final : public engine::deadlock_detector::Actor {
59+
class MockActor final : public engine::impl::deadlock_detector::Actor {
6060
utils::StringLiteral GetActorType() const override { return "mock"; }
6161
};
6262

‎core/src/engine/impl/mutex_impl.hpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
#include <userver/utils/fast_scope_guard.hpp>
99

1010
#include <engine/deadlock_detector.hpp>
11-
#include <engine/deadlock_detector/actor.hpp>
1211
#include <engine/impl/wait_list.hpp>
1312
#include <engine/impl/wait_list_light.hpp>
1413
#include <engine/task/task_context.hpp>
1514
#include <userver/compiler/impl/tsan.hpp>
15+
#include <userver/engine/impl/actor.hpp>
1616

1717
USERVER_NAMESPACE_BEGIN
1818

@@ -22,7 +22,7 @@ template <class Waiters>
2222
class MutexImpl : public deadlock_detector::Actor {
2323
public:
2424
MutexImpl();
25-
~MutexImpl() override;
25+
~MutexImpl();
2626

2727
MutexImpl(const MutexImpl&) = delete;
2828
MutexImpl(MutexImpl&&) = delete;
@@ -162,7 +162,7 @@ void MutexImpl<Waiters>::lock() {
162162

163163
template <class Waiters>
164164
void MutexImpl<Waiters>::unlock() {
165-
auto& dd_state = deadlock_detector::GetState();
165+
auto& dd_state = engine::deadlock_detector::GetState();
166166
dd_state.OnResourceRelease(current_task::GetCurrentTaskContext(), *this);
167167

168168
#if USERVER_IMPL_HAS_TSAN
@@ -198,7 +198,7 @@ bool MutexImpl<Waiters>::try_lock() {
198198
__tsan_mutex_post_lock(this, __tsan_mutex_try_lock | (result ? 0 : __tsan_mutex_try_lock_failed), 0);
199199
#endif
200200

201-
auto& dd_state = deadlock_detector::GetState();
201+
auto& dd_state = engine::deadlock_detector::GetState();
202202
if (result) {
203203
dd_state.OnResourceAcquire(current_task::GetCurrentTaskContext(), *this);
204204
}
@@ -219,7 +219,7 @@ bool MutexImpl<Waiters>::try_lock_until(Deadline deadline) {
219219
});
220220
#endif
221221

222-
std::optional<deadlock_detector::WaitScope> scope;
222+
std::optional<engine::deadlock_detector::WaitScope> scope;
223223
if (!deadline.IsReachable()) {
224224
scope.emplace(*this);
225225
}
@@ -234,7 +234,7 @@ bool MutexImpl<Waiters>::try_lock_until(Deadline deadline) {
234234

235235
scope.reset();
236236

237-
auto& dd_state = deadlock_detector::GetState();
237+
auto& dd_state = engine::deadlock_detector::GetState();
238238
if (result) {
239239
dd_state.OnResourceAcquire(current_task::GetCurrentTaskContext(), *this);
240240
}

‎core/src/engine/task/task_context.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ FutureStatus TaskContext::WaitUntil(Deadline deadline) const noexcept {
182182
static_assert(noexcept(current_task::GetCurrentTaskContext()));
183183
auto& current = current_task::GetCurrentTaskContext();
184184

185-
std::optional<deadlock_detector::WaitScope> scope;
185+
std::optional<engine::deadlock_detector::WaitScope> scope;
186186
if (!deadline.IsReachable()) {
187187
scope.emplace(*this);
188188
}

‎core/src/engine/task/task_context.hpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,12 @@
22

33
#include <atomic>
44
#include <cstdint>
5-
#include <memory>
6-
#include <vector>
75

86
#include <ev.h>
97
#include <boost/intrusive/list_hook.hpp>
108
#include <boost/smart_ptr/intrusive_ref_counter.hpp>
119

1210
#include <engine/coro/pool.hpp>
13-
#include <engine/deadlock_detector/actor.hpp>
1411
#include <engine/ev/thread_control.hpp>
1512
#include <engine/task/context_timer.hpp>
1613
#include <engine/task/counted_coroutine_ptr.hpp>
@@ -19,6 +16,7 @@
1916
#include <engine/task/task_counter.hpp>
2017
#include <userver/engine/deadline.hpp>
2118
#include <userver/engine/future_status.hpp>
19+
#include <userver/engine/impl/actor.hpp>
2220
#include <userver/engine/impl/context_accessor.hpp>
2321
#include <userver/engine/impl/detached_tasks_sync_block.hpp>
2422
#include <userver/engine/impl/task_local_storage.hpp>
@@ -83,7 +81,7 @@ class TaskContext final : public ContextAccessor, public deadlock_detector::Acto
8381

8482
TaskContext(TaskProcessor&, Task::Importance, Task::WaitMode, Deadline, utils::impl::WrappedCallBase& payload);
8583

86-
~TaskContext() noexcept override;
84+
~TaskContext() noexcept;
8785

8886
TaskContext(const TaskContext&) = delete;
8987
TaskContext(TaskContext&&) = delete;

‎core/src/engine/task/task_processor_pools.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ class TaskProcessorPools final {
2020

2121
CoroPool& GetCoroPool() { return coro_pool_; }
2222
ev::ThreadPool& EventThreadPool() { return event_thread_pool_; }
23-
deadlock_detector::State& GetDeadlockDetectorState() { return dd_state_; }
23+
engine::deadlock_detector::State& GetDeadlockDetectorState() { return dd_state_; }
2424

2525
private:
2626
CoroPool coro_pool_;
2727
ev::ThreadPool event_thread_pool_;
28-
deadlock_detector::State dd_state_;
28+
engine::deadlock_detector::State dd_state_;
2929
};
3030

3131
} // namespace engine::impl

0 commit comments

Comments
 (0)