[Rust] Reset apparent_size in Verifier::reset() - #9209
Open
xhon-pelushi wants to merge 1 commit into
Open
Conversation
reset() assigns num_tables twice and never clears apparent_size, so a Verifier
reused through reset() keeps the running total from previous buffers. Once
that total passes max_apparent_size, every later buffer is rejected with
ApparentSizeTooLarge no matter how small or how valid it is - reset() puts the
verifier into a state new() never produces.
Assign apparent_size in place of the duplicated num_tables line, and add a
test that spends most of the budget, resets, and spends it again.
Note: builder::tests::with_internal_capacity_preallocates_vecs already fails
on master ("expected 0 allocations after builder construction, got 2"),
independently of this change.
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
注册 for free
to join this conversation on GitHub.
Already have an account?
登录 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.
Fixes #9189.
Verifier::reset()assignsnum_tablestwice and never clearsapparent_size:apparent_sizeis initialised innew()and incremented inrange_in_buffer(), but cleared nowhere. AVerifierreused throughreset()therefore keeps accumulating across buffers, and once the running total passesmax_apparent_sizeit rejects every subsequent buffer withApparentSizeTooLarge, however small or valid that buffer is. In other wordsreset()leaves the verifier in a statenew()can never produce, which is the opposite of what it is for.Fix
Assign
apparent_sizein place of the duplicatednum_tablesline, plus a test that spends most of the budget, resets, and spends it again. The test is inline as#[cfg(test)] mod tests, matchingbuilder.rs.Against
masterit fails as:Testing
cargo test -p flatbuffers— 7 passed after the change (6 before, plus the new one).tests/rust_usage_test— full suite green: 276 + 21 + 13 and the smaller suites, 0 failures.One thing to be aware of, unrelated to this change:
builder::tests::with_internal_capacity_preallocates_vecsalready fails onmasterwith "expected 0 allocations after builder construction, got 2". I confirmed that by stashing this patch and re-running, so please don't attribute it to this PR.