Skip to content

Commit 33af6f4

Browse files
Drop inherited state in DqliteConnection.close() fork short-circuit
The fork-after-init short-circuit cleared _protocol and _db_id only. The remaining state — _pending_drain (an asyncio.Task bound to the parent's loop), _in_transaction, _tx_owner, _savepoint_stack, _savepoint_implicit_begin, _has_untracked_savepoint, _bound_loop — crossed the fork boundary intact. The Task in particular kept the inherited writer transport alive via its coroutine frame, blocking the kernel from reaping the FD and emitting the "Task was destroyed but it is pending" warning on every forked-worker GC sweep — exactly the warning the parent's _pending_drain-await machinery was built to suppress. Clear all eight items in the fork branch so GC in the child has no references to parent-loop primitives or inherited FDs, and so the child's view of the connection (in_transaction etc.) stays self-consistent for any debugger / metric collector inspecting it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5a49e37 commit 33af6f4

2 files changed

Lines changed: 58 additions & 0 deletions

File tree

src/dqliteclient/connection.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,8 +1097,26 @@ async def close(self) -> None:
10971097
# child preserves that contract for the GC / __del__ path that
10981098
# commonly drives close in a forked worker.
10991099
if os.getpid() != self._creator_pid:
1100+
# Drop every reference that crosses the fork boundary so
1101+
# GC in the child doesn't keep parent-loop primitives or
1102+
# the inherited socket FD alive. ``_pending_drain`` in
1103+
# particular is an ``asyncio.Task`` bound to the parent's
1104+
# loop; if it survives, ``Task.__del__`` later emits
1105+
# "Task was destroyed but it is pending" and the
1106+
# coroutine frame keeps the inherited writer transport
1107+
# (and FD) referenced indefinitely. Clearing the
1108+
# transaction / savepoint / loop bookkeeping also keeps
1109+
# the child's view of the connection self-consistent —
1110+
# ``in_transaction`` reads False for a closed connection.
11001111
self._protocol = None
11011112
self._db_id = None
1113+
self._pending_drain = None
1114+
self._in_transaction = False
1115+
self._tx_owner = None
1116+
self._savepoint_stack.clear()
1117+
self._savepoint_implicit_begin = False
1118+
self._has_untracked_savepoint = False
1119+
self._bound_loop = None
11021120
return
11031121
# Run the in-use guard BEFORE the ``_protocol is None``
11041122
# early-return so a concurrent ``connect()`` racing with

tests/test_connection_after_fork_raises.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import asyncio
2020
import os
21+
from unittest.mock import MagicMock, patch
2122

2223
import pytest
2324

@@ -107,6 +108,45 @@ async def run() -> None:
107108
assert result == b"OK", f"child reported: {result!r}"
108109

109110

111+
def test_dqlite_connection_close_after_fork_drops_inherited_state() -> None:
112+
"""The fork short-circuit in ``DqliteConnection.close()`` must
113+
drop every reference that crosses the fork boundary —
114+
``_pending_drain`` (parent-loop ``asyncio.Task``), transaction
115+
bookkeeping, savepoint stack, and ``_bound_loop`` — so GC in
116+
the child doesn't keep the inherited writer transport / socket
117+
FD alive via the Task's coroutine frame, and the child's
118+
``in_transaction`` view stays self-consistent (False on a
119+
closed connection)."""
120+
conn = DqliteConnection("127.0.0.1:9999")
121+
# Stage state that would normally be cleared by _close_impl.
122+
sentinel_task = MagicMock(spec=asyncio.Task)
123+
conn._pending_drain = sentinel_task
124+
conn._in_transaction = True
125+
conn._tx_owner = MagicMock()
126+
conn._savepoint_stack.append("sp1")
127+
conn._savepoint_implicit_begin = True
128+
conn._has_untracked_savepoint = True
129+
conn._bound_loop = MagicMock()
130+
fake_parent_pid = conn._creator_pid + 1
131+
conn._creator_pid = fake_parent_pid
132+
133+
async def run() -> None:
134+
with patch("dqliteclient.connection.os.getpid", return_value=fake_parent_pid + 1):
135+
await conn.close()
136+
137+
asyncio.run(run())
138+
139+
assert conn._protocol is None
140+
assert conn._db_id is None
141+
assert conn._pending_drain is None
142+
assert conn._in_transaction is False
143+
assert conn._tx_owner is None
144+
assert conn._savepoint_stack == []
145+
assert conn._savepoint_implicit_begin is False
146+
assert conn._has_untracked_savepoint is False
147+
assert conn._bound_loop is None
148+
149+
110150
@pytest.mark.skipif(not hasattr(os, "fork"), reason="requires os.fork")
111151
def test_dqlite_connection_close_after_fork_short_circuits() -> None:
112152
"""``close()`` in the child must not touch the inherited socket

0 commit comments

Comments
 (0)