Skip to content

Commit e272425

Browse files
authored
fix(python-sdk): wait for in-flight cleanup in dispose() (#2892)
1 parent cca682b commit e272425

2 files changed

Lines changed: 65 additions & 8 deletions

File tree

packages/sdk/langs/python/superdoc/transport.py

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -435,24 +435,33 @@ async def connect(self) -> None:
435435
"""Ensure the host process is running and handshake is complete."""
436436
await self._ensure_connected()
437437

438+
async def _await_in_flight_cleanup(self) -> None:
439+
"""Wait for an already-running cleanup task, if any."""
440+
existing = self._cleanup_task
441+
if existing and not existing.done():
442+
try:
443+
await asyncio.shield(existing)
444+
except asyncio.CancelledError:
445+
raise
446+
except Exception:
447+
pass
448+
438449
async def dispose(self) -> None:
439450
"""Gracefully shut down the host process."""
440451
if self._state == _State.DISCONNECTED:
452+
# Reader-triggered cleanup flips state to DISCONNECTED before the
453+
# subprocess is fully reaped. If that cleanup is still in flight,
454+
# wait for it so dispose() doesn't return while the host process
455+
# is still being torn down.
456+
await self._await_in_flight_cleanup()
441457
return
442458
if self._state == _State.DISPOSING:
443459
# A reader-triggered cleanup is in flight (or an earlier teardown
444460
# left state in DISPOSING briefly). Wait for it so the caller
445461
# observes "host fully torn down" by the time dispose() returns.
446462
# shield() so a cancelled dispose() doesn't interrupt _cleanup
447463
# mid-flight and leak the host process.
448-
existing = self._cleanup_task
449-
if existing and not existing.done():
450-
try:
451-
await asyncio.shield(existing)
452-
except asyncio.CancelledError:
453-
raise
454-
except Exception:
455-
pass
464+
await self._await_in_flight_cleanup()
456465
return
457466

458467
self._stopping = True

packages/sdk/langs/python/tests/test_transport.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,54 @@ async def slow_cleanup(error):
764764
finally:
765765
_cleanup_wrapper(cli)
766766

767+
@pytest.mark.asyncio
768+
async def test_dispose_waits_for_cleanup_after_state_flips_disconnected(self):
769+
# `_cleanup()` flips state to DISCONNECTED before awaiting
770+
# `process.wait()`. dispose() must still wait for that cleanup task
771+
# instead of short-circuiting and returning while teardown is in
772+
# flight.
773+
cli = _mock_cli_bin({'handshake': 'ok'})
774+
try:
775+
transport = AsyncHostTransport(cli, startup_timeout_ms=5_000)
776+
await transport.connect()
777+
process = transport._process
778+
assert process is not None
779+
780+
wait_started = asyncio.Event()
781+
release = asyncio.Event()
782+
real_wait = process.wait
783+
784+
async def slow_wait():
785+
wait_started.set()
786+
await release.wait()
787+
return await real_wait()
788+
789+
process.wait = slow_wait # type: ignore[assignment]
790+
791+
transport._schedule_cleanup(
792+
SuperDocError('reader-overflow', code=HOST_PROTOCOL_ERROR),
793+
)
794+
cleanup_task = transport._cleanup_task
795+
assert cleanup_task is not None
796+
797+
await asyncio.wait_for(wait_started.wait(), timeout=2.0)
798+
assert transport.state == 'DISCONNECTED'
799+
assert transport._process is None
800+
assert not cleanup_task.done()
801+
802+
dispose_task = asyncio.create_task(transport.dispose())
803+
await asyncio.sleep(0.05)
804+
assert not dispose_task.done()
805+
806+
release.set()
807+
await dispose_task
808+
assert transport.state == 'DISCONNECTED'
809+
assert transport._cleanup_task is None
810+
await process.wait()
811+
assert process.returncode is not None
812+
finally:
813+
_cleanup_wrapper(cli)
814+
767815
@pytest.mark.asyncio
768816
async def test_ensure_connected_drains_in_flight_cleanup_before_spawn(self):
769817
# Round-3 regression: without this drain, `_start_host` reassigns

0 commit comments

Comments
 (0)