Skip to content

Commit ed30c94

Browse files
authored
fix(executor): prevent self-join deadlock in ~ExecutorImpl (alibaba#1562)
When the destructor is triggered on the executor's own worker thread (e.g. via shared_ptr cascade from a task), pthread_join(self) returns EDEADLK -> std::system_error -> escapes noexcept dtor -> terminate. Fix: detect self-destruction via worker_id comparison, detach instead of join, and spin-wait for running flag before returning. Add test_self_join.cpp with synchronous (promise/future) verification.
1 parent c300327 commit ed30c94

3 files changed

Lines changed: 67 additions & 3 deletions

File tree

common/executor/executor.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class Executor::ExecutorImpl {
1919
using CBList =
2020
common::RingChannel<LockfreeMPMCRingQueue<Delegate<void>, 32UL * 1024>>;
2121
std::unique_ptr<std::thread> th;
22+
std::atomic<bool> running{false};
2223
photon::thread *pth = nullptr;
2324
CBList queue;
2425
photon::ThreadPoolBase *pool;
@@ -34,10 +35,18 @@ class Executor::ExecutorImpl {
3435

3536
~ExecutorImpl() {
3637
queue.send({});
37-
if (th)
38-
th->join();
39-
else
38+
if (th) {
39+
if (std::this_thread::get_id() == th->get_id()) {
40+
// Self-destruction: called from within the worker thread.
41+
th->detach();
42+
while (running) photon::thread_yield();
43+
} else {
44+
th->join();
45+
}
46+
} else {
4047
while (pool) photon::thread_yield();
48+
}
49+
4150
}
4251

4352
struct CallArg {
@@ -68,6 +77,8 @@ class Executor::ExecutorImpl {
6877
}
6978

7079
void do_loop() {
80+
running = true;
81+
DEFER(running = false);
7182
pth = photon::CURRENT;
7283
pool = photon::new_thread_pool(32);
7384
LOG_INFO("worker start");

common/executor/test/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,7 @@ add_executable(test-executor-burst-drain test_burst_drain.cpp)
2626
target_link_libraries(test-executor-burst-drain PRIVATE photon_shared ${GOOGLETEST_GTEST_MAIN_LIBRARIES})
2727
add_test(NAME test-executor-burst-drain COMMAND $<TARGET_FILE:test-executor-burst-drain>)
2828

29+
add_executable(test-executor-self-join test_self_join.cpp)
30+
target_link_libraries(test-executor-self-join PRIVATE photon_shared ${GOOGLETEST_GTEST_MAIN_LIBRARIES})
31+
add_test(NAME test-executor-self-join COMMAND $<TARGET_FILE:test-executor-self-join>)
32+
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
Copyright 2022 The Photon Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Verify ~ExecutorImpl won't self-join (EDEADLK → terminate) when
18+
// destroyed from its own worker thread. Related: #967, #969.
19+
20+
#include <photon/common/executor/executor.h>
21+
#include <photon/common/alog.h>
22+
#include <photon/photon.h>
23+
#include "../../../test/gtest.h"
24+
25+
#include <memory>
26+
#include <atomic>
27+
#include <future>
28+
29+
using namespace photon;
30+
31+
// Task releases the last shared_ptr ref on the worker thread,
32+
// triggering ~ExecutorImpl from within → must detach, not join.
33+
TEST(executor, destroy_from_worker_thread) {
34+
std::promise<void> done;
35+
auto fut = done.get_future();
36+
37+
{
38+
auto exec = std::make_shared<Executor>(
39+
photon::INIT_EVENT_DEFAULT, photon::INIT_IO_NONE);
40+
41+
exec->async_perform(new auto([&exec, &done] {
42+
exec.reset(); // ~Executor on worker thread
43+
done.set_value(); // reachable only if no crash
44+
}));
45+
}
46+
47+
fut.wait();
48+
LOG_INFO("executor destroyed from worker thread without crash");
49+
}

0 commit comments

Comments
 (0)