Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 90 additions & 9 deletions BlueprintUI/Sources/BlueprintView/BlueprintView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ public final class BlueprintView: UIView {

/// Used to detect reentrant updates
private var isInsideUpdate: Bool = false
private var hasScheduledDeferredViewHierarchyUpdate: Bool = false
#if DEBUG
private var consecutiveDeferredViewHierarchyUpdateCount = 0
private static let maximumConsecutiveDeferredViewHierarchyUpdateCount = 10
#endif

private let rootController: NativeViewController

Expand Down Expand Up @@ -364,13 +369,82 @@ public final class BlueprintView: UIView {
invalidateIntrinsicContentSize()
sizesThatFit.removeAll()

if needsViewHierarchyUpdate { return }
if needsViewHierarchyUpdate {
if isInsideUpdate {
scheduleDeferredViewHierarchyUpdate()
}

return
}

needsViewHierarchyUpdate = true

/// We use `UIView`'s layout pass to actually perform a hierarchy update.
/// If a manual update is required, call `layoutIfNeeded()`.
setNeedsLayout()
if isInsideUpdate {
scheduleDeferredViewHierarchyUpdate()
} else {
setNeedsLayout()
}
}

/// Schedules a coalesced view hierarchy update on a following main run loop turn.
///
/// Used when an invalidation arrives while an update is already in progress — for example,
/// UIKit keyboard and safe-area callbacks can fire synchronously mid-update and force a
/// layout pass. Re-arming with `setNeedsLayout()` immediately would let a forcing
/// `layoutIfNeeded()` re-dirty this view while the in-flight update still holds
/// `isInsideUpdate`, preventing that forced layout from ever terminating, so the re-arm
/// is deferred until the current update has unwound.
@MainActor
private func scheduleDeferredViewHierarchyUpdate() {
guard hasScheduledDeferredViewHierarchyUpdate == false else { return }

hasScheduledDeferredViewHierarchyUpdate = true
recordDeferredViewHierarchyUpdate()

Task { @MainActor [weak self] in
guard let self else { return }

hasScheduledDeferredViewHierarchyUpdate = false

guard needsViewHierarchyUpdate else {
resetDeferredViewHierarchyUpdateCount()
return
}

setNeedsLayout()
}
}

private func recordDeferredViewHierarchyUpdate() {
Logger.logDeferredViewHierarchyUpdate(view: self)

#if DEBUG
consecutiveDeferredViewHierarchyUpdateCount += 1

guard consecutiveDeferredViewHierarchyUpdateCount >= Self.maximumConsecutiveDeferredViewHierarchyUpdateCount else {
return
}

Logger.logExcessiveDeferredViewHierarchyUpdates(
view: self,
count: consecutiveDeferredViewHierarchyUpdateCount
)

assertionFailure(
"""
BlueprintView deferred \(consecutiveDeferredViewHierarchyUpdateCount) consecutive reentrant view hierarchy updates.
Check for view callbacks that synchronously invalidate and force layout during Blueprint view updates.
"""
)
#endif
}

private func resetDeferredViewHierarchyUpdateCount() {
#if DEBUG
consecutiveDeferredViewHierarchyUpdateCount = 0
#endif
}

@MainActor
Expand All @@ -386,10 +460,13 @@ public final class BlueprintView: UIView {

guard needsViewHierarchyUpdate || bounds != lastViewHierarchyUpdateBounds else { return }

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
}
Comment on lines -389 to +469

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging this case could be useful, if we'd like to track this going forward.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this look? bd980ae


isInsideUpdate = true

Expand Down Expand Up @@ -457,13 +534,18 @@ public final class BlueprintView: UIView {

/// We intentionally deliver our lifecycle callbacks (eg, `onAppear`,
/// `onDisappear`, etc, _after_ we've marked our view as updated.
/// This is in case the `onAppear` callback triggers a re-render,
/// we don't hit our recurisve update precondition.
/// This way, if an `onAppear` callback triggers a re-render, it is
/// applied synchronously on the next layout pass rather than being
/// treated as a reentrant update and deferred.

for callback in updateResult.lifecycleCallbacks {
callback()
}

if hasScheduledDeferredViewHierarchyUpdate == false {
resetDeferredViewHierarchyUpdateCount()
}

Logger.logViewUpdateEnd(view: self)
let viewUpdateEndTime = CACurrentMediaTime()

Expand Down Expand Up @@ -830,4 +912,3 @@ extension BlueprintView.NativeViewController {
}
}
}

25 changes: 25 additions & 0 deletions BlueprintUI/Sources/Internal/Logger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,31 @@ extension Logger {
)
}

static func logDeferredViewHierarchyUpdate(view: BlueprintView) {

guard BlueprintLogging.isEnabled else { return }

os_signpost(
.event,
log: .active,
name: "Deferred View Hierarchy Update",
signpostID: OSSignpostID(log: .active, object: view),
"Deferred reentrant view hierarchy update for %{public}s",
view.name ?? "BlueprintView"
)
}

static func logExcessiveDeferredViewHierarchyUpdates(view: BlueprintView, count: Int) {

os_log(
.fault,
log: .blueprint,
"BlueprintView deferred %{public}d consecutive reentrant view hierarchy updates for %{public}s",
count,
view.name ?? "BlueprintView"
)
}

static func logSizeThatFitsStart(
view: BlueprintView,
description: @autoclosure () -> String
Expand Down
153 changes: 153 additions & 0 deletions BlueprintUI/Tests/BlueprintViewTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,145 @@ class BlueprintViewTests: XCTestCase {
XCTAssertTrue(layoutRecursed)
}

func test_safeAreaChangeDuringUpdateDefersNestedUpdate() {
let viewController = UIViewController()
let view = BlueprintView(frame: CGRect(x: 0, y: 0, width: 390, height: 844))

viewController.view.frame = view.bounds
viewController.view.addSubview(view)

let window = UIWindow(frame: view.bounds)
window.rootViewController = viewController
window.makeKeyAndVisible()

let safeAreaInvalidation = SafeAreaInvalidation()

struct SafeAreaInvalidatingElement: Element {
weak var viewController: UIViewController?
let safeAreaInvalidation: SafeAreaInvalidation

var content: ElementContent {
ElementContent(measuring: Spacer(width: 1, height: 1))
}

func backingViewDescription(with context: ViewDescriptionContext) -> ViewDescription? {
SafeAreaInvalidatingView.describe { config in
config.builder = {
SafeAreaInvalidatingView(
frame: context.bounds,
viewController: viewController,
safeAreaInvalidation: safeAreaInvalidation
)
}

config.apply { view in
view.viewController = viewController
view.safeAreaInvalidation = safeAreaInvalidation
}
}
}
}

final class SafeAreaInvalidation {
var hasInvalidatedSafeArea = false
}

final class SafeAreaInvalidatingView: UIView {
weak var viewController: UIViewController?
var safeAreaInvalidation: SafeAreaInvalidation

init(
frame: CGRect,
viewController: UIViewController?,
safeAreaInvalidation: SafeAreaInvalidation
) {
self.viewController = viewController
self.safeAreaInvalidation = safeAreaInvalidation
super.init(frame: frame)
}

required init?(coder: NSCoder) { fatalError() }

override func didMoveToWindow() {
super.didMoveToWindow()
invalidateSafeAreaOnce()
}

override func layoutSubviews() {
super.layoutSubviews()
invalidateSafeAreaOnce()
}

private func invalidateSafeAreaOnce() {
guard
safeAreaInvalidation.hasInvalidatedSafeArea == false,
window != nil,
let viewController
else {
return
}

safeAreaInvalidation.hasInvalidatedSafeArea = true

viewController.additionalSafeAreaInsets.bottom += 1
viewController.view.setNeedsLayout()
viewController.view.layoutIfNeeded()
}
}

view.element = SafeAreaInvalidatingElement(
viewController: viewController,
safeAreaInvalidation: safeAreaInvalidation
)
view.layoutIfNeeded()
}

func test_elementChangeAndForcedLayoutDuringUpdateDefersNestedUpdate() {
let view = BlueprintView(frame: CGRect(x: 0, y: 0, width: 200, height: 200))

var didTriggerReentrantUpdate = false

// Mirrors the navigation-bar crash shape: while the view is mid-update, a view
// callback replaces `element` and forces a synchronous layout pass.
view.element = CallbackElement {
guard !didTriggerReentrantUpdate else { return }
didTriggerReentrantUpdate = true

view.element = IdentifiedElement(identifier: "replacement")
view.setNeedsLayout()
view.layoutIfNeeded()
}

view.layoutIfNeeded()

XCTAssertTrue(didTriggerReentrantUpdate)

// The deferred update applies the replacement element on a following
// main run loop turn.
let deadline = Date(timeIntervalSinceNow: 5)

while findView(withAccessibilityIdentifier: "replacement", in: view) == nil, Date() < deadline {
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.01))
view.layoutIfNeeded()
}

XCTAssertNotNil(findView(withAccessibilityIdentifier: "replacement", in: view))
}

private func findView(withAccessibilityIdentifier identifier: String, in root: UIView) -> UIView? {
if root.accessibilityIdentifier == identifier {
return root
}

for subview in root.subviews {
if let match = findView(withAccessibilityIdentifier: identifier, in: subview) {
return match
}
}

return nil
}

func test_baseEnvironment() {
enum TestValue {
case defaultValue
Expand Down Expand Up @@ -846,6 +985,20 @@ private struct TestContainer: Element {
}
}

private struct IdentifiedElement: Element {
var identifier: String

var content: ElementContent {
ElementContent(intrinsicSize: .zero)
}

func backingViewDescription(with context: ViewDescriptionContext) -> ViewDescription? {
UIView.describe { config in
config[\.accessibilityIdentifier] = identifier
}
}
}

private struct CallbackElement: Element {
var onViewDescriptionApplied: () -> Void

Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- `BlueprintView` now defers view hierarchy updates that are triggered while an update is already in progress, instead of crashing with a "Reentrant updates are not supported" precondition failure. Reentrant invalidations (e.g. keyboard or safe-area changes that synchronously force a layout pass mid-update) are coalesced and applied on a following main run loop turn.

### Added

### Removed
Expand Down
5 changes: 4 additions & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ let package = Package(
),
.testTarget(
name: "BlueprintUITests",
dependencies: ["BlueprintUI"],
dependencies: [
"BlueprintUI",
"BlueprintUICommonControls",
],
path: "BlueprintUI/Tests"
),
.target(
Expand Down
Loading