Skip to content

Bound aiosendspin_server.close() timeout to stop harness SIGKILLs - #96

Merged
OnFreund merged 1 commit into
mainfrom
fix-aiosendspin-server-close-hang
Jul 18, 2026
Merged

Bound aiosendspin_server.close() timeout to stop harness SIGKILLs#96
OnFreund merged 1 commit into
mainfrom
fix-aiosendspin-server-close-hang

Conversation

@OnFreund

Copy link
Copy Markdown
Contributor

Summary

client-initiated-pcm (aiosendspin server, sendspin-jvm client) fails in CI, but both sides actually succeed at the protocol level — the client exits 0 and audio hashes match in both summaries. The failure is caused by the aiosendspin server process hanging during shutdown after the result is already recorded, and getting SIGKILL'd by the harness's 30s timeout. Because str(asyncio.TimeoutError()) is "", the runner reports this as a failure with an empty reason.

Root cause is a real defect in aiosendspin, filed separately: Sendspin/aiosendspin#299. SendspinServer.close() runs an unbounded asyncio.gather(*disconnect_tasks, ...) for _clients-tracked (client-initiated) connections, where each task calls SendspinConnection.disconnect(), which cancels self._message_loop_task and awaits it with no timeout. Two of the three cleanup phases in close() already use asyncio.timeout(1.0); this one doesn't.

This PR is a harness-side workaround, not a fix for the library bug: it wraps the adapter's await server.close() call in _run()'s finally block with asyncio.wait_for(..., timeout=5.0), logging and continuing on timeout instead of letting it hang. By the time close() runs, the scenario summary has already been written (success or error path), so a slow/hung close() is pure post-result cleanup and shouldn't fail an otherwise-passing case.

This surfaced via Sendspin/sendspin-jvm#25's CI run.

Test plan

Verified locally against the exact sibling-repo layout CI uses (conformance, aiosendspin, sendspin-cli, sendspin-jvm), built the sendspin-jvm-client adapter JAR, and ran conformance run --from aiosendspin --to sendspin-jvm:

  • Before the fix: client-initiated-pcm fails with server_exit_code: -9 (harness SIGKILL after timeout waiting on the server).

  • After the fix: client-initiated-pcm passes (PCM hashes match exactly, server_exit_code: 0). The server log shows WARNING:root:server.close() timed out after 5s; continuing shutdown — confirming the underlying aiosendspin hang still occurs, but the adapter now recovers instead of being killed.

  • Confirmed no regression: all previously-passing scenarios (server-initiated-pcm, server-initiated-metadata, server-initiated-artwork, server-initiated-controller, server-initiated-flac, server-initiated-pcm-24bit) still pass, and their server.log files show no timeout warning — close() still completes immediately for non-client-initiated connections, so this is a no-op for those paths.

  • python -m compileall src passes.

  • Reproduced the failure locally

  • Verified the fix resolves it without touching other scenarios

  • Not run against CI yet (this PR)

🤖 Generated with Claude Code

aiosendspin's SendspinServer.close() can hang indefinitely for
client-initiated connections: it awaits an unbounded
asyncio.gather(*disconnect_tasks) where each disconnect() cancels the
connection's message-loop task with no timeout on the cancellation await
(Sendspin/aiosendspin#299). By the time close() runs in the adapter's
finally block, the scenario summary is already written and the run has
already succeeded or hit its error path, so a hang here is pure
post-result cleanup that shouldn't fail an otherwise-passing case. Wrap
it in a 5s asyncio.wait_for so the adapter process exits on its own
instead of requiring the harness's 30s SIGKILL.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@OnFreund
OnFreund merged commit 68a4620 into main Jul 18, 2026
2 checks passed
@OnFreund
OnFreund deleted the fix-aiosendspin-server-close-hang branch July 18, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant