Reject negative chunk puts and keep pending HT consistent on NOMEM - #2533
Merged
Conversation
chunkStorePut hashed and copied nData before checking sign, so a negative size wrapped the write-buf grow and memcpy. Return SQLITE_TOOBIG like PutSparse. Pending hash-table grow assigned nPendingHTSize before the next-index realloc. A NOMEM there left a larger mask over uninitialized buckets. Commit the new size only after both reallocs succeed, then rebuild. Cover both with chunk_store_put_bounds_test: negative puts stay out of the store, and a failed HT grow still finds previously inserted pending chunks. Co-Authored-By: Grok 4.6 <noreply@x.ai>
|
SummaryCoverage spans normal storage and retrieval, duplicate handling, empty and invalid input, persistence across close and reopen, and behavior at large pending-table boundaries. It also exercises adversarial memory-allocation failures and recovery, plus test-selection and feature-toggle behavior, with healthy results across these areas. Safe to merge — the run found no regressions, new failures, or previously reported failures attributable to this PR, and the exercised storage behaviors remain healthy. No merge blocker is indicated; overall risk is low. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
Contributor
DoltLite performance vs PR base
blobpk details
compositepk details
int details
textpk details
vc details
All relative performance gates passed. |
Contributor
DoltLite source coverage
Merged 202 pooled raw profiles from the distributed Linux correctness jobs. Per-file coverage (98 files)
|
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
1 participant
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.
Beta review item 4: two storage-engine footguns.
Negative
chunkStorePutsize.nDatawas hashed and copied before any sign check. A negative length wrapsCS_WAL_CHUNK_HDR_SIZE + nDataas a signed int and is undefined inmemcpy. Reject withSQLITE_TOOBIG, matchingchunkStorePutSparse.Pending hash-table grow on
NOMEM.csPendHTEnsuresetnPendingHTSizeto the new mask before reallocatingaPendingHTNext. If that realloc failed, later lookups used the larger mask over uninitialized buckets and treated garbage as next-indexes. Assign the new size only after both reallocs succeed, then rebuild.test/chunk_store_put_bounds_test.c(coverage C suite):Co-Authored-By: Grok 4.6 noreply@x.ai