Fix duplicate typed-array buffer release - #10609
Merged
Merged
Conversation
Greptile SummaryThis PR makes typed-array cleanup idempotent by clearing the owned allocation reference immediately after release.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| modules/core/src/lib/attribute/data-column.ts | Clears allocatedValue after returning it to the typed-array pool, preventing repeated deletion from releasing the same allocation twice. |
| test/modules/core/lib/attribute/attribute.spec.ts | Covers idempotent deletion and keeps shared test attributes alive until all update-buffer parameter cases finish. |
Reviews (2): Last reviewed commit: "Merge branch 'master' into codex/attribu..." | Re-trigger Greptile
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.
Stack position: Merged core prerequisite below #10607. It was squash-merged into
masteras99a5aea70c; #10607 has since synchronized currentmasterthrough an append-only merge.Goal
Make attribute cleanup idempotent so a deleted attribute cannot return the same CPU buffer to the global typed-array pool twice.
Changes
DataColumn.state.allocatedValueimmediately after releasing it.Benchmark results
No draw-time benchmark applies:
delete()is a teardown path and this PR intentionally does not change rendering or steady-state layer updates. The measured benefit is correctness: repeated deletion no longer releases the same pooled typed array twice, preventing aliasing between later attribute allocations.Validation
yarn vitest run --project headless test/modules/core/lib/attribute/attribute.spec.ts— 33/33 passed.yarn biome check modules/core/src/lib/attribute/data-column.ts test/modules/core/lib/attribute/attribute.spec.ts.yarn tsc --noEmit -p modules/core/tsconfig.json.Note
Low Risk
Teardown-only change in attribute cleanup with no steady-state rendering impact; risk is limited to incorrect double-release behavior that this PR fixes.
Overview
DataColumn.delete()now clearsstate.allocatedValueright after returning the CPU typed array to the global pool, so a seconddelete()cannot release the same backing buffer twice and cause later attribute allocations to alias pooled memory.Tests use
allocate(4)instead ofsetDatafor the delete case, assert thatallocatedValueis cleared (including after a repeated delete), and tear down sharedupdateBufferfixtures once per test case after all parameter variants run.Reviewed by Cursor Bugbot for commit bba16a2. Bugbot is set up for automated code reviews on this repo. Configure here.