Repository navigation
Conversation
An allocation failure inside an HLL sketch or union could leak memory, leave a sketch pointing to freed memory, or leave the sketch or union changed or internally inconsistent. Allocate objects through a helper that keeps the allocator's pointer type until construction succeeds and frees storage if the constructor throws. Use it for construction, copies, conversions, promotions and deserialization. Use copy-and-swap for sketch and union assignment, including self-assignment. Roll back LIST, SET and AuxHashMap insertions when growth fails, and update the auxiliary table size only after the larger table is in place. Copy auxiliary maps with an explicit source allocator for both the object and its entries. Preserve table order even when container copy selection would choose an unequal allocator. Move LIST and SET growth and promotion, with their rollback, into non-inlined functions so the ordinary coupon path stays small. Mark the sketch constructor and object helper inline to retain inexpensive LIST construction. In HLL_4, create and fill the auxiliary map before writing AUX_TOKEN. Keep auxiliary updates out of the inlined register path. Check for the last minimum register before changing it. On the ordinary non-auxiliary update path, update the estimator before the non-allocating register writes. Validate and decrement registers in one pass per curMin step, restoring the written registers if a consistency check or allocation throws. Keep replacement auxiliary maps owned until they are installed. During conversion, use the same update path with the iterator's slot directly; the delegating constructor cleans up if conversion throws. Merge sparse source coupons through a local implementation pointer, updating the union's owning pointer after each promotion. Retain the union's allocator for inputs with unequal allocators, including guarded HLL copies and downsampling at the smaller precision. Sweep those paths with per-allocator accounting and check source-pool lifetimes. The sketch documentation now states: "Allocation failures propagate without leaking storage. Updates, copy assignment and reset leave the sketch unchanged if an allocation fails." The union documentation now states: "Allocation failures propagate without leaking storage. If an allocation fails while a LIST or SET sketch is merged into a union in LIST or SET mode, the union stays valid and usable, holding the coupons inserted before the failure. Repeating the merge gives the result of an uninterrupted merge. All other updates, copy assignment and reset leave the union unchanged if an allocation fails." This also fixes a heap buffer overflow when serializing after failed auxiliary growth, and a use-after-free when stream deserialization finds the wrong number of auxiliary entries. Reject overfull compact SET images before replaying their coupons, so replay cannot lose a promoted implementation. Check the SET precision before computing the compact count limit. Add allocation-failure sweeps that check cleanup, unchanged destinations where guaranteed, continued use and merge retries. Compare allocator pointer and raw-pointer results, and check construction-failure cleanup. Check null construction, assignment and comparisons in both operand orders. Exercise selecting allocators on curMin shifts, conversions and failures, recording mismatched releases and balancing each pool. Check that rejected curMin shifts on malformed HLL_4 images leave the sketch unchanged. Preserve the original consistency exceptions where both implementations reject an image. A curMin shift also rejects HLL_4 images that mark a slot as an exception without an auxiliary entry; the previous code accepted them. Fixes apache#551 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5852c4a7-7589-49ef-bf80-8f2050501a9d
## Summary Remove two unreachable HLL_4 branches and two duplicate allocation-failure test cases. Explain the minimum-shift entry condition and map lifetime. ## Prompting Intent Reduce unnecessary changes while preserving the allocation-failure fix, its guarantees and distinct allocator coverage. ## Linked Sources - apache#565 - apache#551 ## Rationale The auxiliary update caller excludes ordinary values, and the shift scan already rejects exception tokens without a map. Existing sketch-update sweeps cover union scalar forwarding; failed-mutation checks retain their serialization assertions. Keep the outlined first-map helper and the multi-shift transaction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b7960e30-8d3c-4554-82c5-881b981ab3f4
clee704
marked this pull request as draft
November 5, 2026 00:22
clee704
marked this pull request as ready for review
November 5, 2026 01:08
proost
reviewed
Nov 6, 2026
| std::memcpy(&couponCount, data + hll_constants::HASH_SET_COUNT_INT, sizeof(couponCount)); | ||
| const uint32_t maxCouponCount = hll_constants::RESIZE_NUMER * (uint32_t(1) << (lgK - 3)) | ||
| / hll_constants::RESIZE_DENOM; | ||
| if (compactFlag && couponCount > maxCouponCount) { |
This branch has not been deployed
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.
Fixes #551.
Prevent allocation failures from leaking HLL storage or leaving sketches and
unions unusable. Guard unfinished objects, roll back failed insertions, and
commit HLL_4 auxiliary-map changes only after allocation succeeds.
Sketch updates, copy assignment and reset leave the sketch unchanged on
allocation failure. Merging LIST/SET sketches into LIST/SET unions may retain
coupons already inserted; the union remains usable and the merge can be
retried. Other union updates, copy assignment and reset leave it unchanged.
Also fix self-assignment and malformed auxiliary-map deserialization cleanup.
Reject overfull compact SET images and orphan HLL_4 exception tokens.
Tests inject allocation failures across construction, updates, copying,
conversion, deserialization and union merges, checking cleanup, rollback and
allocator ownership. Reported sanitizer and C++/Java serialization comparisons
pass.
Performance: Previously measured shared-host timing increases:
Some Clang SET-assignment batches reached about 19%. These observations are
not bounds; timing causes remain unverified.
This change was developed with assistance from GitHub Copilot.