Skip to content

Commit dadbb86

Browse files
committed
fix(Session): avoid mutex deadlock in DeleteAllSessions
Calling database_ref_.Reset() can trigger GC, which can finalize other Session objects. Those Session destructors call Delete() -> RemoveSession() which tries to lock sessions_mutex_. Since std::mutex is not recursive, holding the lock during Reset() causes undefined behavior (SIGSEGV on Alpine/musl). Fix: Release the mutex before iterating. Since sessions_ is cleared first, any RemoveSession() calls during GC become no-ops (erasing from empty set).
1 parent fb283df commit dadbb86

File tree

1 file changed

+13
-5
lines changed

1 file changed

+13
-5
lines changed

src/sqlite_impl.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,12 +1514,19 @@ void DatabaseSync::RemoveSession(Session *session) {
15141514
}
15151515

15161516
void DatabaseSync::DeleteAllSessions() {
1517-
std::lock_guard<std::mutex> lock(sessions_mutex_);
1518-
// Copy the set to avoid iterator invalidation
1519-
std::set<Session *> sessions_copy = sessions_;
1520-
sessions_.clear(); // Clear first to prevent re-entrance
1517+
// Copy and clear sessions while holding the lock, then release lock
1518+
// before calling Reset() which can trigger GC. GC can finalize other
1519+
// Session objects which call Delete() -> RemoveSession() which also
1520+
// tries to lock sessions_mutex_. Since std::mutex is not recursive,
1521+
// we must release the lock before any operation that could trigger GC.
1522+
std::set<Session *> sessions_copy;
1523+
{
1524+
std::lock_guard<std::mutex> lock(sessions_mutex_);
1525+
sessions_copy = sessions_;
1526+
sessions_.clear(); // Clear first so RemoveSession() becomes a no-op
1527+
}
15211528

1522-
// Now delete each session
1529+
// Now delete each session WITHOUT holding the mutex
15231530
for (auto *session : sessions_copy) {
15241531
// Direct SQLite cleanup since we're in database destruction
15251532
if (session->GetSession()) {
@@ -1531,6 +1538,7 @@ void DatabaseSync::DeleteAllSessions() {
15311538
}
15321539
// Release the database reference now rather than waiting for Session
15331540
// destructor. This ensures cleanup happens while environment is valid.
1541+
// This can trigger GC which may finalize other Session objects.
15341542
if (!session->database_ref_.IsEmpty()) {
15351543
session->database_ref_.Reset();
15361544
}

0 commit comments

Comments
 (0)