fix: rethrow non-longjmp exceptions in execProtocolRawSync protocol loop - #1097
Closed
Brown-Sage wants to merge 1 commit into
Closed
fix: rethrow non-longjmp exceptions in execProtocolRawSync protocol loop#1097Brown-Sage wants to merge 1 commit into
Brown-Sage wants to merge 1 commit into
Conversation
When the WASM backend terminates while execProtocolRawSync is processing a message (e.g. exit(1) after hitting EOF during a COPY ... FROM STDIN), _PostgresMainLoopOnce() throws ExitStatus. That is not the longjmp sentinel, so it was silently swallowed; the dead backend consumes no input, the loop condition never becomes false, and the call spins forever, synchronously, at 100% CPU — unkillable by timers, Promise.race or AbortSignal. Rethrow anything that is not the longjmp sentinel so callers see the backend error and follow-up queries fail fast. Guard the finally's own WASM calls so they cannot mask the original error against a dead runtime. Fixes electric-sql#1058
Author
|
Closing in favor of #1081 which takes a more conservative approach to the same fix (narrower rethrow scope to preserve extension behavior, plus exitCode restoration). Thanks @psh4607 — one note: a RuntimeError thrown when |
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.
Description
Fixes #1058.
When the WASM backend terminates while
execProtocolRawSyncis processing a message (e.g.exit(1)after hitting EOF during aCOPY ... FROM STDINissued viaexec()), the protocol loop spins forever, synchronously, at 100% CPU. The call never settles, so no timer,Promise.race, orAbortSignalcan interrupt it — the process has to be killed from outside.Root cause
After the backend exits,
_PostgresMainLoopOnce()throwsExitStatus. That is not the longjmp sentinel, so it was silently swallowed; the dead backend consumes no input, so#readOffsetnever advances and_pq_buffer_remaining_data()never drains — thewhilecondition is permanently true, producing a tight call → throw → swallow loop.Changes
ExitStatus, WASMRuntimeError, …) — the backend is gone; looping cannot helpfinally's own WASM calls (_PostgresSendReadyForQueryIfNecessary/_pgl_pq_flush) so they cannot mask the original error against a dead runtimeBehavior after the fix (verified against the repro from #1058)
exec()rejects withExitStatus { status: 1 }close()resolves and the event loop stays alive throughoutRegression test
New
tests/backend-death.test.ts:COPY ... FROM STDINrejects instead of hangingclose()still resolvesAgainst unfixed code the test process hangs synchronously — it even defeats vitest's own test timeout, so the baseline run has to be killed externally. With this patch all tests pass.
Also adds a changeset for
@electric-sql/pglite.Fixes #1058