drogonframework / drogonframework/drogon

EventLoopThread self-join in DbClientImpl teardown when last ref released from own DbLoop thread (throws Resource deadlock avoided)

Open
#2,552 3 comments 0 reactions 0 assignees View on GitHub
bug Focus good first issue
Dominant language
C++
Stars
14.3k
Forks
1.4k
Avg merge
1d 13h
Merged PRs (30d)
14

Description

**Notice**
If you need support or clarification regarding the usage of Drogon in
your project, visit the official Drogon support channel at
[gitter](https://gitter.im/drogon-web/community)

Please create a new issue only if you think you have found a bug or if
have a feature request/enhancement.

**Describe the bug**

`trantor::EventLoopThread::~EventLoopThread()` calls `thread_.join()` on
its own worker thread as part of normal teardown. If the *last*
`shared_ptr` reference is released from
*inside a callback already executing on that same DbLoop thread* (in our
reproduction: `PgConnection::handleRead()` → `DbClientImpl::makeTrans`'s
cleanup lambda), the resulting destructor chain
(`~DbClientImpl` → `~EventLoopThreadPool` → `~EventLoopThread`) runs
*on* the thread it's about to try to join. `std::thread::join()`
correctly detects this self-join
(`get_id() == std::this_thread::get_id()`) and throws
`std::system_error(std::errc::resource_deadlock_would_occur)` —
uncaught, since nothing in `DbClientImpl`'s teardown path expects a
destructor to throw, which aborts the process.

This looks related to #1657 (open since 2023) — same general shape
("something torn down from inside its own event-loop callback thread"),
but a different concrete code path (that one is `HttpClient`/
`sendRequest`/`app.quit()` and hangs; this one is `DbClientImpl`/
`makeTrans` and throws) — filing separately in case the root fix isn't
the same, but flagging the connection in case it is (e.g. a common
underlying pattern of "this class doesn't guard against being torn down
from its own worker thread").

**To Reproduce**

Minimal repro (no HTTP/WebSocket/ORM-schema, no application code at all
— just `DbClient` construct/destroy in a loop, against any reachable
PostgreSQL instance):

```cpp
#include
#include
#include
#include

int main() {
std::thread app_thread([]() { drogon::app().run(); });
while (!drogon::app().isRunning())
std::this_thread::sleep_for(std::chrono::milliseconds(10));

// Repeatedly construct and destroy a DbClient. Each iteration has a
// chance of hitting the exact interleaving where the last reference
// is released from the DbLoop thread's own callback.
for (int i = 0; i < 50; ++i) {
auto client = drogon::orm::DbClient::newPgClient(
"host=127.0.0.1 port=5432 dbname=postgres user=postgres", 1);
std::this_thread::sleep_for(std::chrono::milliseconds(50));
try {
client->execSqlSync("SELECT 1");
} catch (...) {}
client.reset(); // <- crash can happen here or shortly after
}

drogon::app().quit();
app_thread.join();
return 0;
}
```

This exact ~50-line program, compiled standalone (just Drogon + trantor +
libpq — no ORM schema, no HTTP/WebSocket layer, no other application
code) and run against a fresh, empty PostgreSQL instance, **crashed on
its first run**:

```
terminate called after throwing an instance of 'std::system_error'
what(): Resource deadlock avoided
```

It does not crash on every run — two subsequent runs of the same binary
completed cleanly, consistent with a narrow timing window — but no
larger application or test suite is needed to hit it; this minimal
harness alone is sufficient.

Separately, we've also hit the same failure via a larger real test suite
that does `MigrationRunner`-style work (open connection, run several
statements inside a `newTransaction()`, let the `Transaction` destruct,
then destroy the `DbClient`) in a tight loop across multiple
`TEST_F`-style test cases — `ctest`-driven reruns of that suite: 2
crashes in ~35 runs. Caught live there with:

```
gdb -batch -ex "catch throw std::system_error" -ex run \
-ex "thread apply all bt" --args ./your_test_binary
```

in a retry loop (took ~24 attempts to catch) — that's where the
backtrace below comes from.

**Expected behavior**

`DbClientImpl`'s teardown should not be able to reach
`EventLoopThread::~EventLoopThread()`'s `join()` while executing on that
same `EventLoopThread`'s own thread — either by deferring the actual
thread-pool teardown to a different thread, or by having
`EventLoopThread`'s destructor detect and handle the self-join case
instead of leaving `std::thread::join()`'s exception to propagate
uncaught out of a destructor.

**Backtrace** (trimmed to the load-bearing frames; full trace available
on request)

```
Thread "DbLoop" hit Catchpoint (exception thrown), __cxa_throw ()
#1 std::__throw_system_error(int) ()
#3 trantor::EventLoopThread::~EventLoopThread (this=...) at EventLoopThread.cc:45
#6 std::_Sp_counted_ptr_inplace::_M_dispose
#16 trantor::EventLoopThreadPool::~EventLoopThreadPool
#17 drogon::orm::DbClientImpl::~DbClientImpl (this=...) at DbClientImpl.cc:102
#26 operator() (__closure=...) at DbClientImpl.cc:312 [makeTrans's cleanup lambda]
#31 drogon::orm::PgConnection::handleRead (this=...) at PgBatchConnection.cc:486
#37 trantor::Channel::handleEventSafely
#39 trantor::EventLoop::loop (this=...) at EventLoop.cc:234
#40 trantor::EventLoopThread::loopFuncs (this=...) at EventLoopThread.cc:65
```

Line numbers are from Drogon 1.9.13 (vcpkg port-version 1) / trantor
1.5.28 (vcpkg), unmodified upstream source — none of vcpkg's build
patches for these ports touch any of the files above (only
`CMakeLists.txt`/config templates are patched).

**Desktop (please complete the following information)**
- OS: Ubuntu (WSL2), also expected on native Linux — nothing WSL2-specific
in the mechanism
- Drogon version: 1.9.13 (port-version 1, vcpkg)
- trantor version: 1.5.28
- Compiler: GCC 15.2.0, `-fsanitize=thread` build (though this is not a
TSan-detected race — it reproduces the same way in a non-instrumented
build, just without a debugger attached to catch it live)

**Additional context**

Not a ThreadSanitizer finding — this is a real, uncaught C++ exception
that aborts the process, independent of any sanitizer. We initially
suspected it might be a manifestation of a known Drogon/trantor
TSan-flagged cross-thread synchronization gap we'd been separately
investigating (`TransactionImpl`/`SqlBinder`/`MpscQueue`), but confirmed
via `gdb` that this is a structurally different failure: a genuine
self-join, not a missing-synchronization race.

Observed rate: the minimal repro above hit it on a first run, and a
larger test suite that exercises this code path far more often (once per
test case, many test cases) hit it roughly 1 in 15-35 runs. We suspect
the actual trigger condition is narrow timing (whether `handleRead`'s
cleanup path happens to be the one holding the last reference at the
moment of destruction) rather than any particular code shape — happy to
help narrow this down further if a maintainer can point at which code
path is most likely to hit the exact interleaving.

Contributor guide

Open the contributing guide

Research direction

Start with trantor/EventLoopThread.cc and drogon/orm/DbClientImpl.cc, then trace the reported cleanup path through PgBatchConnection.cc. Run the minimal standalone PostgreSQL reproducer and inspect the teardown backtrace. Done means destroying the client from its callback path no longer reaches a self-join or aborts with Resource deadlock avoided.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, postgresql
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.