Repository navigation
Release linked sketch caches during UI cleanup - #1817
Merged
Merged
Conversation
Contributor
Author
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the linked-sketch cache cleanup change.
No open issues 🎉
What changed in this PR
Releases linked-sketch caches during UI cleanup to address the order-dependent shutdown leak reported in #1813.
Changes:
- Calls
ClearLinkedSketches()fromSolveSpaceUI::Clear(), covering application/CLI shutdown and test teardown.
| File | Description |
|---|---|
src/solvespace.cpp |
Adds explicit linked-sketch cache cleanup to SolveSpaceUI::Clear(). |
Review effort: Balanced
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.
SolveSpaceUI::Clear()leaves the linked-sketch caches populated. Their cachedSketchobjects are owned through raw pointers and need the explicit cleanup already provided byClearLinkedSketches(). If a linked-file test runs last, shutdown leaks those objects and their allocations. Starting another sketch clears the caches, which makes the failure depend on the shuffled test order.Call
ClearLinkedSketches()during UI cleanup, covering application/CLI shutdown and test teardown.This addresses the Ubuntu sanitizer failure reported in #1813.
group/link/normal_roundtripalone reproduces the same 45,428-byte leak in 17 allocations on masterabf33ac. It also reproduces with the two helix files restored to their pre-#1813 versions fromb34d744, so the leak predates that PR.Tested in a Debug headless build with GCC 11.4,
ENABLE_SANITIZERS=ON,ENABLE_OPENMP=ON, leak detection enabled and three OpenMP threads. The isolated reproducer and all 300 tests (1,246 checks) pass without sanitizer errors after the fix. The full suite used seed2062209549from the failing CI run; before the fix it reproduces the same leak. No interactive GUI testing was performed.