fix(datagrid): crash adding a row while a cell overlay is open (#1378)#1379
Merged
Conversation
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.
Fixes #1378.
Problem
Adding a row (Cmd+Shift+N) crashes when a cell editor or value viewer overlay is open. The trace is a Swift exclusive-access-to-memory violation (
swift_beginAccess->AccessSet::insert->_swift_runtime_on_report), not a stack overflow.Cause
KeyHandlingTableView.didAddSubviewpassed the reentrancy guard asinout(flag: &isRaisingOverlayEditor), which holds an exclusive access for the whole call. Inside,raiseToFront()callsaddSubview, which synchronously re-entersdidAddSubviewand tries to take a second overlapping access to the same property. The runtime traps. Theguard !flagnever gets to run because the conflict is detected when theinoutargument is formed.Regression from #1341, which refactored the prior inline guard into an
inouthelper. The version before that used a direct instance-property guard, which does not conflict.Fix
Drop the
inoutparameter. Use oneisRaisingOverlayguard hoisted to the top ofdidAddSubview, reset withdefer. Bool reads and writes are instantaneous accesses, so the reentrant call short-circuits cleanly. Editor and viewer are mutually exclusive (eachshowdismisses the other), so a single guard is correct.Test
KeyHandlingTableViewOverlayTestsinstalls an active overlay, adds another subview to force the re-raise path, and asserts the overlay ends on top. This path traps before the fix and passes after. Verified locally with** TEST SUCCEEDED **.Not included
The reporter also mentioned "the plus button to add a new row is not visible." That has no trace and is not explained by this crash. Tracking separately.