fix(decoders): report completion from terminal states instead of stalling - #124
Merged
Merged
Conversation
…ling Follow-up to #123. That fix covered deflate/deflate64/zlib; reviewing all 53 `raw_decode` implementations found the same defect in seven more. A decoder parked in its terminal state that returns `done: false` with nothing consumed and nothing written, while the caller still holds input, is mapped by the RawDecoder->Decoder bridge to `Status::OutputFull` — "call me again" — with nothing to progress on. Any loop waiting for StreamEnd spins: CPU-bound, no allocation, so no output or memory cap catches it. Fixed, each verified by reading its state machine and confirmed with a probe that drives the real encoder output plus trailing bytes: - zstd, xz Done arms returned false; both are terminal (neither supports concatenated frames/streams). - lz4 block, lzo Done after the zero-length terminator block. - lz4 frame only reachable by calling past StreamEnd, but the state is terminal either way. - lzx, amiga_lzx Done arms returned false. - rar5 returns on `State::Done` *before* the block that accepts caller bytes, so a container passing the next header after the payload could never make progress. - gzip subtler: the `BetweenMembers` arm sets `phase = Done` intending to reach the Done arm that swallows trailing bytes, but match arms do not fall through, so the loop's no-progress check returned first and the swallow never ran. Needs an explicit `continue`, like the 0x1F path. Two call shapes matter and only the first was covered before: trailing bytes in the same call, and trailing bytes arriving *after* the payload completes. The second is what a container does when it hands over the next header, and it is what xz and gzip failed — both looked safe under the first shape. Codecs that only ever report `InputEmpty` and produce output on `finish` (brotli, lzma, and the other buffering decoders) are correct as-is: the trait permits that, and it terminates. The invariant asserted is not "must report StreamEnd" but "must never report OutputFull with zero progress". A sweep over all 50 round-trippable algorithms now reports no stalls. tests/terminal_state.rs covers each fixed codec plus controls; rar5 is decoder-only so it gets equivalent tests against its own fixture.
MagicalTux
force-pushed
the
fix/decoder-terminal-state-stall
branch
from
August 16, 2026 21:30
62f36b5 to
8ecc1a9
Compare
Merged
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.
Follow-up to #123, which fixed this class of bug in deflate/deflate64/zlib. Reviewing all 53
raw_decodeimplementations found the same defect in seven more codecs.The defect
A decoder parked in its terminal state that returns
done: falsewith nothing consumed and nothing written — while the caller still holds input — is mapped by theRawDecoder→Decoderbridge toStatus::OutputFull, i.e. "drain and call me again", with nothing left to progress on. Any caller looping untilStreamEndspins: CPU-bound, no allocation, so neither an output-size cap nor a memory cap can stop it.Fixed
zstd,xzDonearms returnedfalse; both terminal (neither supports concatenated frames/streams)lz4block,lzoDoneafter the zero-length terminator blocklz4frameStreamEnd, but the state is terminal either waylzx,amiga_lzxDonearms returnedfalserar5State::Donebefore the block that accepts caller bytes — a container passing the next header after the payload could never make progressgzipgzipis the subtle one. ItsBetweenMembersarm setsphase = Doneintending to reach theDonearm that swallows trailing bytes (added precisely to avoid this spin). But match arms do not fall through, so the loop's no-progress check returned first and the swallow never ran. It needs an explicitcontinue, like the adjacent0x1Fpath already has.Two call shapes
Only the first was covered before:
xzandgzippass shape 1 and failed shape 2; both looked safe until the second shape was tested. Tests cover both.What is not a bug
Codecs that only ever report
InputEmptyand produce output onfinish(brotli, lzma, and the other buffering decoders) are correct as-is — the trait permits it and it terminates. The invariant asserted is therefore not "must report StreamEnd" but "must never report OutputFull with zero progress".Verification
Each fix was found by reading the state machine and confirmed with a probe driving real encoder output plus trailing bytes. A sweep over all 50 round-trippable algorithms now reports no stalls.
tests/terminal_state.rscovers every fixed codec plus controls; rar5 is decoder-only so it gets equivalent tests against its own fixture.cargo test --all-features: 1766 pass, 0 fail.fmtandclippy -D warningsclean.