[APP-7B2] [DO NOT MERGE] Reproduce and validate iOS deadlock fix in ComponentDescriptorRegistry#91607
[APP-7B2] [DO NOT MERGE] Reproduce and validate iOS deadlock fix in ComponentDescriptorRegistry#91607JakubKorytko wants to merge 6 commits into
ComponentDescriptorRegistry#91607Conversation
|
|
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
This comment has been minimized.
This comment has been minimized.
This reverts commit 6d39405.
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
DO NOT MERGE - APP-7B2 Deadlock Reproduction & Fix Validation
This PR exists solely to reproduce and validate the fix for the APP-7B2 fatal iOS app hang described in #91292. It is not intended for merge.
How to test
This branch has two commits. Each should be tested with a separate ad-hoc build:
simulate app-7B2 crash- demonstrates the deadlock (app hangs)fix app-7B2 crash- applies the fix (app no longer hangs)Steps:
What is APP-7B2?
A fatal iOS app hang caused by mutex contention in React Native Fabric's
ComponentDescriptorRegistry. It occurs during OldDot -> NewDot transitions in HybridApp when legacy interop components are lazily registered.The deadlock (two threads, circular dependency)
stepAnimations)shared_lock(mutex_)infindComponentDescriptorByHandle_DO_NOT_USE_THIS_IS_BROKEN_registerComponentIfPossible)unique_lock(mutex_)inComponentDescriptorRegistry::add()RCTUnsafeExecuteOnMainQueueSync(insideLegacyViewManagerInteropComponentDescriptorconstructor)The main thread can't acquire
shared_lockbecause the background thread holdsunique_lock. The background thread can't proceed because it's waiting for the main thread (which is blocked). Classic deadlock.Commit 1: Reproduce (
+037patch)Patch:
react-native+0.83.1+037+deadlock-repro-APP-7B2.patchAdds an
RCTDeadlockRepronative module that recreates the exact conditions from the Sentry crash report:CADisplayLinkrepeatedly callsfindComponentDescriptorByHandle_DO_NOT_USE_THIS_IS_BROKEN, acquiringshared_lock(mutex_)- matching Sentry Thread 0's stackComponentDescriptorRegistry::add()(acquiresunique_lock(mutex_)) ->LegacyViewManagerInteropComponentDescriptorconstructor ->RCTUnsafeExecuteOnMainQueueSync- matching Sentry Thread 24's stackThe repro also wraps the original
getViewManagerClass()call inRCTUnsafeExecuteOnMainQueueSyncinsideconstructCoordinator()to force the main-queue dependency that exists in production (where view managers must be accessed on the main thread).Key implementation details:
ComponentDescriptorProviderobjects (not just names) to preserveflavorshared_ptrs across re-registration cycles, keepingComponentHandlevalues stableclearProvidersForTesting) to only remove legacy interop providers, leaving standard component providers intact for subsequentComponentDescriptorRegistrycreationFiles modified:
RCTComponentViewFactory.mm- addsclearDescriptorCacheForTestingandcallBrokenLookupForTestingComponentDescriptorProviderRegistry.cpp/.h- addsclearForTesting,clearProvidersForTesting,callBrokenLookupForTestingComponentDescriptorRegistry.cpp/.h- adds logging andretainCurrentDescriptorsLegacyViewManagerInteropComponentDescriptor.mm- wrapsgetViewManagerClassinRCTUnsafeExecuteOnMainQueueSyncRCTDeadlockRepro.mm/.h- the native module orchestrating the reproductionHybridAppHandler.tsx- triggers the repro on OldDot->NewDot transitionCommit 2: Fix (
+038patch)Patch:
react-native+0.83.1+038+fix-deadlock-APP-7B2.patchMoves the descriptor construction outside the
unique_lockinComponentDescriptorRegistry::add(). The lock is only acquired for the final map insertions (_registryByHandle,_registryByName):This breaks the circular dependency: the background thread no longer holds
unique_lockwhen callingRCTUnsafeExecuteOnMainQueueSync, so the main thread can acquireshared_lockfreely.Videos
Simulated (commit 1)
Simulated.mov
Simulated with fix (commit 2)
Fixed.mov