fix(cron): cancel orphan coroutine on delivery timeout before standalone fallback (salvage #13495)#13517
Merged
fix(cron): cancel orphan coroutine on delivery timeout before standalone fallback (salvage #13495)#13517
Conversation
…one fallback When the live adapter delivery path (_deliver_result) or media send path (_send_media_via_adapter) times out at future.result(timeout=N), the underlying coroutine scheduled via asyncio.run_coroutine_threadsafe can still complete on the event loop, causing a duplicate send after the standalone fallback runs. Cancel the future on TimeoutError before re-raising, so the standalone fallback is the sole delivery path. Adds TestDeliverResultTimeoutCancelsFuture and TestSendMediaTimeoutCancelsFuture.
…ctly for timeout-cancel The original tests replicated the try/except/cancel/raise pattern inline with a mocked future, which tested Python's try/except semantics rather than the scheduler's behavior. Rewrite them to invoke _deliver_result and _send_media_via_adapter end-to-end with a real concurrent.futures.Future whose .result() raises TimeoutError. Mutation-verified: both tests fail when the try/except wrappers are removed from cron/scheduler.py, pass with them in place.
8 tasks
alt-glitch
added
type/bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On cron delivery timeout, the live-adapter coroutine is now cancelled — no more duplicate sends from the orphan coroutine completing on the event loop after the standalone fallback already delivered.
Salvage of #13495 by @VTRiot. Cherry-picked onto current main; tests rewritten to invoke the real functions.
Changes
cron/scheduler.py:_deliver_result(timeout=60) and_send_media_via_adapter(timeout=30) wrapfuture.result()intry/except TimeoutError: future.cancel(); raise. Mirrors the existing precedent intools/mcp_tool.py:1513.tests/cron/test_scheduler.py: rewritten. The original tests replicated the try/except/cancel pattern inline against a mocked future — testing Python's semantics, not the scheduler. Replaced with tests that call_deliver_resultand_send_media_via_adapterdirectly using a realconcurrent.futures.Futurewhose.result()raisesTimeoutError, and assert cancel() fires plus downstream behavior (standalone fallback delivers, next media file still dispatched).scripts/release.py: register VTRiot in AUTHOR_MAP (VTRiot's own commit).Validation
Mutation test: reverted both try/except wrappers in
cron/scheduler.py→ both new tests fail withassert [] == [True]. Restored → pass.Why it matters more after #13021
#13021made cron jobs run in parallel viaThreadPoolExecutor. Multiple in-flight delivery coroutines on the same event loop increases exposure to this orphan-coroutine bug.Closes #13495.