Release runtime keepalive in emscripten_clear_timeout and make clear_timeout/clear_immediate idempotent - #27720
Merged
Conversation
sbc100
reviewed
Sep 16, 2026
guybedford
force-pushed
the
timeout-keepalive
branch
2 times, most recently
from
September 16, 2026 19:50
4a4a2ec to
88ad1ef
Compare
sbc100
reviewed
Sep 16, 2026
sbc100
reviewed
Sep 16, 2026
sbc100
approved these changes
Sep 16, 2026
guybedford
enabled auto-merge (squash)
September 16, 2026 20:12
guybedford
force-pushed
the
timeout-keepalive
branch
from
September 16, 2026 22:11
4d3142a to
f1ae5c3
Compare
…timeout/clear_immediate idempotent
guybedford
force-pushed
the
timeout-keepalive
branch
from
September 16, 2026 23:11
f1ae5c3 to
a8ce995
Compare
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
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.
Resolves #23763.
This fixes a runtime keepalive leak in
emscripten_clear_timeoutand makesemscripten_clear_timeout/emscripten_clear_immediatesafe to call for ids that have already fired.emscripten_set_timeoutgoes throughsafeSetTimeout, which doesruntimeKeepalivePush()and pops in the fire wrapper.emscripten_clear_timeoutwas aliased straight toclearTimeout, so clearing a pending timer leaked one keepalive forever: withEXIT_RUNTIME=1the runtime never exits aftermainreturns (noatexithandlers run,emscripten_runtime_keepalive_check()stays true).emscripten_clear_immediatehad the mirror bug: it popped unconditionally, so clearing an already-fired immediate double-popped the counter.safeSetTimeoutnow follows thesetImmediateWrapped.mappingpattern: it returns an i32 index intosafeSetTimeout.mappingholding the native handle, and the fire wrapper clears the slot before popping the keepalive. Ids start at 1 so, likesetTimeout, they are never zero.$safeClearTimeout(id)pops and clears only if the slot is still live, otherwise it is a no-op.emscripten_clear_timeoutaliases it, which also removes theAUDIO_WORKLETspecial case.SDL_PauseAudiouses it too instead of a rawclearTimeout, which had the same leak.emClearImmediate(both thesetImmediateandpostMessagevariants) reports whether the immediate was still pending, andemscripten_clear_immediatepops only in that case.Tests:
other.test_emscripten_clear_timeout_{fires,cleared,idempotent,immediate}cover clearing a pending timeout (runtime exits frommainimmediately withatexitrunning), clearing the same id twice and clearing an id after it fired while another pending timer keeps the runtime alive, the existing set-then-fire behaviour, and the same idempotency for immediates. Three of the four fail without the fix.Made with AI assistance under my review