Handle missing viewState gracefully in SurfaceMountingManager.updateState and updatePadding (#57181)#57181
Closed
shubhamksavita wants to merge 1 commit into
Closed
Handle missing viewState gracefully in SurfaceMountingManager.updateState and updatePadding (#57181)#57181shubhamksavita wants to merge 1 commit into
shubhamksavita wants to merge 1 commit into
Conversation
ffce5d0 to
a804982
Compare
|
@shubhamksavita has exported this pull request. If you are a Meta employee, you can view the originating Diff in D107768754. |
…tate and updatePadding (#57181) Summary: ## Summary Fixes `RetryableMountingLayerException: Unable to find viewState for tag N. Surface stopped: false` crashing `com.oculus.firsttimenux` from the Fabric batch-mount path. Logview link: [252c85116a7ab5c4ec93ef3c3373cf9d](https://www.internalfb.com/logview/system_vros_crashes/252c85116a7ab5c4ec93ef3c3373cf9d) ## Root cause `IntBufferBatchMountItem.execute` dispatches batched mount instructions. When a view tag is transiently missing from the registry (async race between the JS commit and native mount — surface teardown / out-of-order delivery), mount methods that call the throwing `getViewState` raise `RetryableMountingLayerException`. `MountItemDispatcher.dispatchMountItems` only retries this exception `if (item is DispatchCommandMountItem)` (comment: *"Only DispatchCommandMountItem supports retries"*) — `IntBufferBatchMountItem` is not one, so the "retryable" exception propagates uncaught and crashes the app. The RN team has been hardening each batch mount method to handle missing viewState gracefully (soft-log + early return): `addViewAt` in `D99760257` (which references this same MID and process), `updateOverflowInset` in `D104400233`; `removeViewAt`, `updateProps`, `updateLayout`, and `deleteView` were already graceful. The originally-reported `addViewAt` stack is therefore already fixed in trunk — crashes persist on release branch v201/v203 which predates `D99760257`. `updateState` (`INSTRUCTION_UPDATE_STATE`) and `updatePadding` (`INSTRUCTION_UPDATE_PADDING`) were the two remaining throwing `getViewState` callers reachable from `IntBufferBatchMountItem.execute` — unfixed siblings of the same multi-site pattern and live/contributing crash sites for the same exception. ## Fix Change `updateState` and `updatePadding` to use `getNullableViewState`; on null, log a `SURFACE_MOUNTING_MANAGER_MISSING_VIEWSTATE` soft exception and return early — identical to the established pattern in `addViewAt`, `updateOverflowInset`, `updateProps`, `updateLayout`, and `deleteView`. This is not a throw-downgrade of an invariant: `RetryableMountingLayerException` is an explicitly retryable transient signal, and this matches the canonical handling the RN team applies to every other batch mount method. The non-batch callers (`sendAccessibilityEvent`, `setJSResponder`) are intentionally left unchanged — `sendAccessibilityEvent` is already protected by its own `try/catch (RetryableMountingLayerException)` in `SendAccessibilityEventMountItem.execute`. ## Changelog: [Android] [Fixed] - Handle missing viewState gracefully in `SurfaceMountingManager.updateState` and `updatePadding` to avoid `RetryableMountingLayerException` crashes from the Fabric batch-mount path Reviewed By: javache Differential Revision: D107768754
a804982 to
cecd11e
Compare
|
This pull request has been merged in 0e86a04. |
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.
Summary:
Summary
Fixes
RetryableMountingLayerException: Unable to find viewState for tag N. Surface stopped: falsecrashingcom.oculus.firsttimenuxfrom the Fabric batch-mount path.Logview link: 252c85116a7ab5c4ec93ef3c3373cf9d
Root cause
IntBufferBatchMountItem.executedispatches batched mount instructions. When a view tag is transiently missing from the registry (async race between the JS commit and native mount — surface teardown / out-of-order delivery), mount methods that call the throwinggetViewStateraiseRetryableMountingLayerException.MountItemDispatcher.dispatchMountItemsonly retries this exceptionif (item is DispatchCommandMountItem)(comment: "Only DispatchCommandMountItem supports retries") —IntBufferBatchMountItemis not one, so the "retryable" exception propagates uncaught and crashes the app.The RN team has been hardening each batch mount method to handle missing viewState gracefully (soft-log + early return):
addViewAtinD99760257(which references this same MID and process),updateOverflowInsetinD104400233;removeViewAt,updateProps,updateLayout, anddeleteViewwere already graceful. The originally-reportedaddViewAtstack is therefore already fixed in trunk — crashes persist on release branch v201/v203 which predatesD99760257.updateState(INSTRUCTION_UPDATE_STATE) andupdatePadding(INSTRUCTION_UPDATE_PADDING) were the two remaining throwinggetViewStatecallers reachable fromIntBufferBatchMountItem.execute— unfixed siblings of the same multi-site pattern and live/contributing crash sites for the same exception.Fix
Change
updateStateandupdatePaddingto usegetNullableViewState; on null, log aSURFACE_MOUNTING_MANAGER_MISSING_VIEWSTATEsoft exception and return early — identical to the established pattern inaddViewAt,updateOverflowInset,updateProps,updateLayout, anddeleteView. This is not a throw-downgrade of an invariant:RetryableMountingLayerExceptionis an explicitly retryable transient signal, and this matches the canonical handling the RN team applies to every other batch mount method.The non-batch callers (
sendAccessibilityEvent,setJSResponder) are intentionally left unchanged —sendAccessibilityEventis already protected by its owntry/catch (RetryableMountingLayerException)inSendAccessibilityEventMountItem.execute.Changelog:
[Android] [Fixed] - Handle missing viewState gracefully in
SurfaceMountingManager.updateStateandupdatePaddingto avoidRetryableMountingLayerExceptioncrashes from the Fabric batch-mount pathReviewed By: javache
Differential Revision: D107768754