fix: defer reentrant BlueprintView updates#618
Conversation
Add a regression test mirroring the navigation-bar crash shape from production: a view callback replaces `element` and forces a synchronous layout pass while the view hierarchy update is still in progress. The test crashes on the reentrant-update precondition without the deferral fix, and additionally asserts that the deferred update converges (the replacement element's backing view is applied on a following main run loop turn). Also add the CHANGELOG entry, document why the deferred re-arm is required (an immediate setNeedsLayout re-arm would prevent a forcing layoutIfNeeded from terminating while the in-flight update still holds isInsideUpdate), and update the lifecycle-callback comment that still referenced the removed precondition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
johnnewman-square
left a comment
johnnewman-square
left a comment
There was a problem hiding this comment.
This is the best fix proposal so far. This should address all call sites, more than the isolated Market fix.
I'm in favor of trying this out until we find a tighter repro.
| precondition( | ||
| !isInsideUpdate, | ||
| "Reentrant updates are not supported in BlueprintView. Ensure that view events from within the hierarchy are not synchronously triggering additional updates." | ||
| ) | ||
| if isInsideUpdate { | ||
| // A reentrant update was triggered from within the in-flight update, e.g. by a | ||
| // safe-area or keyboard change synchronously forcing a layout pass. Defer it so | ||
| // the current update can finish; the deferred pass picks up the latest state. | ||
| setNeedsViewHierarchyUpdate() | ||
| return | ||
| } |
There was a problem hiding this comment.
Logging this case could be useful, if we'd like to track this going forward.
There was a problem hiding this comment.
To avoid infinite async layout churn, an agent suggested this:
I'd suggest a debug-only diagnostic — e.g., count consecutive deferred re-arms with no intervening quiet turn and assertionFailure/os_log past a threshold (say 10). That preserves the developer signal the precondition used to provide while keeping production safe.
I think there's value in logging and in debug asserts for cases like this.
johnnewman-square
left a comment
johnnewman-square
left a comment
There was a problem hiding this comment.
The new logging changes look great. Thanks for doing that.
2 participants
Summary
updateViewHierarchyIfNeeded()additionalSafeAreaInsetsduring backing-view attach/layout, matching the production crash shapeBlueprintUICommonControlsdependency for the package test target so the focused package test buildsContext
This came from an ios-register 7.10 crash investigation where
BlueprintView.safeAreaInsetsDidChange()re-enteredBlueprintView.updateViewHierarchyIfNeeded()during a Market keyboard/header layout update. A MarketUI harness reproduced the exact precondition crash locally before this Blueprint patch; with this patch copied into Market's Blueprint checkout, the same harness passes.Testing
xcodebuild test -scheme BlueprintUI-Package -destination 'platform=iOS Simulator,name=iPhone 17 Pro,OS=26.5' -only-testing:BlueprintUITests/BlueprintViewTests/test_safeAreaChangeDuringUpdateDefersNestedUpdate -quietxcodebuild test -workspace ios/MarketDevelopment.xcworkspace -scheme UnitTests -destination 'platform=iOS Simulator,name=iPhone 17 Pro,OS=26.5' -only-testing:MarketUI-Tests/MarketKeyboardReaderReentrantLayoutTests/test_safeAreaChangeDuringKeyboardReaderUpdateDoesNotReenterBlueprint -quiet