[iOS] Fixes main thread stuck when reload in bridgeless mode by zhongwuzw · Pull Request #45486 · react/react-native

zhongwuzw · 2024-07-17T03:36:12Z

Summary:

In fabric bridgeless mode, when we reload, main thread may block because of dead lock. the backtrace example as below:

(lldb) bt
* thread #1, stop reason = signal SIGSTOP
* frame #0: 0x000000010a5c76f2 libsystem_kernel.dylib`__psynch_mutexwait + 10
frame #1: 0x00000001099e0a70 libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_wait + 78
frame #2: 0x00000001099de82b libsystem_pthread.dylib`_pthread_mutex_firstfit_lock_slow + 217
frame #3: 0x00007ff80030c6b9 libc++.1.dylib`std::__1::mutex::lock() + 9
frame #4: 0x0000000106968b13 RNTester`std::__1::lock_guard<std::__1::mutex>::lock_guard[abi:ue170006](this=0x00007ff7b95e2478, __m=0x000060000377c958) at lock_guard.h:35:10
frame #5: 0x00000001069689ed RNTester`std::__1::lock_guard<std::__1::mutex>::lock_guard[abi:ue170006](this=0x00007ff7b95e2478, __m=0x000060000377c958) at lock_guard.h:34:19
frame #6: 0x00000001070691c1 RNTester`-[RCTInstance invalidate](self=0x000060000377c900, _cmd="invalidate") at RCTInstance.mm:146:31
frame #7: 0x0000000107060fd2 RNTester`-[RCTHost didReceiveReloadCommand](self=0x0000600003d100f0, _cmd="didReceiveReloadCommand") at RCTHost.mm:317:3
frame #8: 0x0000000106b005a5 RNTester`RCTTriggerReloadCommandListeners(reason=@"Global hotkey") at RCTReloadCommand.m:57:5
frame #9: 0x0000000106b86da5 RNTester`__28-[RCTDevSettings initialize]_block_invoke.157(.block_descriptor=0x0000000107496170, params=0x00007ff84002f610) at RCTDevSettings.mm:201:11
frame #10: 0x0000000106ae658e RNTester`__65-[RCTPackagerConnection reconnectingWebSocket:didReceiveMessage:]_block_invoke.68(.block_descriptor=0x0000600000c82df0) at RCTPackagerConnection.mm:293:9
frame #11: 0x0000000109a4529d libdispatch.dylib`_dispatch_call_block_and_release + 12
frame #12: 0x0000000109a4658f libdispatch.dylib`_dispatch_client_callout + 8
frame #13: 0x0000000109a563ee libdispatch.dylib`_dispatch_main_queue_drain + 1362
frame #14: 0x0000000109a55e8e libdispatch.dylib`_dispatch_main_queue_callback_4CF + 31
frame #15: 0x00007ff800429af4 CoreFoundation`__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
frame #16: 0x00007ff80042442f CoreFoundation`__CFRunLoopRun + 2463
frame #17: 0x00007ff8004236ad CoreFoundation`CFRunLoopRunSpecific + 557
frame #18: 0x00007ff8103da08f GraphicsServices`GSEventRunModal + 137
frame #19: 0x00007ff805cc0ad1 UIKitCore`-[UIApplication _run] + 972
frame #20: 0x00007ff805cc5551 UIKitCore`UIApplicationMain + 123
frame #21: 0x00000001069205a0 RNTester`main(argc=1, argv=0x00007ff7b95e3b60) at main.m:15:12
frame #22: 0x00000001099023e0 dyld_sim`start_sim + 10
frame #23: 0x0000000116b92366 dyld`start + 1942
(lldb) bt
* thread #3
frame #0: 0x000000010a5c6b86 libsystem_kernel.dylib`__ulock_wait + 10
frame #1: 0x0000000109a46eb1 libdispatch.dylib`_dlock_wait + 46
frame #2: 0x0000000109a46d08 libdispatch.dylib`_dispatch_thread_event_wait_slow + 40
frame #3: 0x0000000109a5774a libdispatch.dylib`__DISPATCH_WAIT_FOR_QUEUE__ + 371
frame #4: 0x0000000109a57161 libdispatch.dylib`_dispatch_sync_f_slow + 240
frame #5: 0x0000000106b3b33b RNTester`RCTUnsafeExecuteOnMainQueueSync(block=0x0000000106f116c0) at RCTUtils.m:291:5
* frame #6: 0x0000000106f115ad RNTester`-[RCTFabricSurface start](self=0x000000010af0df40, _cmd="start") at RCTFabricSurface.mm:102:3
frame #7: 0x00000001070601ce RNTester`__108-[RCTHost initWithBundleURLProvider:hostDelegate:turboModuleManagerDelegate:jsEngineProvider:launchOptions:]_block_invoke_2(.block_descriptor=0x0000600000c75590) at RCTHost.mm:211:9
frame #8: 0x000000010706cdc8 RNTester`-[RCTInstance _loadScriptFromSource:](self=0x000060000377c900, _cmd="_loadScriptFromSource:", source=0x0000600000cd57d0) at RCTInstance.mm:472:5
frame #9: 0x000000010706ca81 RNTester`__29-[RCTInstance _loadJSBundle:]_block_invoke.120(.block_descriptor=0x0000600000c96d00, error=0x0000000000000000, source=0x0000600000cd57d0) at RCTInstance.mm:452:9
frame #10: 0x0000000106ab1919 RNTester`invocation function for block in attemptAsynchronousLoadOfBundleAtURL(.block_descriptor=0x00006000017b0fc0, statusCode=200, headers=6 key/value pairs, data=0x00006000002a4760, error=0x0000000000000000, done=YES) block_pointer, void (NSError*, RCTSource*) block_pointer) at RCTJavaScriptLoader.mm:318:9
frame #11: 0x0000000106ad92a6 RNTester`__80-[RCTMultipartDataTask URLSession:streamTask:didBecomeInputStream:outputStream:]_block_invoke(.block_descriptor=0x000070000035c7a0, headers=6 key/value pairs, content=0x00006000002a4760, done=YES) at RCTMultipartDataTask.m:121:9
frame #12: 0x0000000106ad9b4f RNTester`-[RCTMultipartStreamReader emitChunk:headers:callback:done:](self=0x00006000002b4220, _cmd="emitChunk:headers:callback:done:", data=0x00006000002a4020, headers=6 key/value pairs, callback=0x0000000106ad9230, done=YES) at RCTMultipartStreamReader.m:57:5
frame #13: 0x0000000106ada800 RNTester`-[RCTMultipartStreamReader readAllPartsWithCompletionCallback:progressCallback:](self=0x00006000002b4220, _cmd="readAllPartsWithCompletionCallback:progressCallback:", callback=0x0000000106ad9230, progressCallback=0x0000000106ab2a60) at RCTMultipartStreamReader.m:154:7
frame #14: 0x0000000106ad9130 RNTester`-[RCTMultipartDataTask URLSession:streamTask:didBecomeInputStream:outputStream:](self=0x00006000017b0d40, _cmd="URLSession:streamTask:didBecomeInputStream:outputStream:", session=0x000000010e02a0a0, streamTask=0x000000010c83ba00, inputStream=0x000060000300d4d0, outputStream=0x000060000300c990) at RCTMultipartDataTask.m:119:20
frame #15: 0x00007ff80479fdf9 CFNetwork`___lldb_unnamed_symbol2876 + 42
frame #16: 0x0000000109a4529d libdispatch.dylib`_dispatch_call_block_and_release + 12
frame #17: 0x0000000109a4658f libdispatch.dylib`_dispatch_client_callout + 8
frame #18: 0x0000000109a4e4ba libdispatch.dylib`_dispatch_lane_serial_drain + 1127
frame #19: 0x0000000109a4f255 libdispatch.dylib`_dispatch_lane_invoke + 441
frame #20: 0x0000000109a5c356 libdispatch.dylib`_dispatch_root_queue_drain_deferred_wlh + 318
frame #21: 0x0000000109a5b751 libdispatch.dylib`_dispatch_workloop_worker_thread + 590
frame #22: 0x00000001099dfb84 libsystem_pthread.dylib`_pthread_wqthread + 327
frame #23: 0x00000001099deacf libsystem_pthread.dylib`start_wqthread + 15

Changelog:

[IOS] [FIXED] - Fixes main thread stuck when reload in bridgeless mode

Test Plan:

RNTester, enables fabric, which is very easy to repro by tapping r command multiple times quickly to trigger reload.

analysis-bot · 2024-07-17T04:16:00Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	21,353,613	-1,243
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	24,549,459	-532
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: fcd526d
Branch: main

cipolleschi

Hi @zhongwuzw, thanks for the fix. I left a question/suggestion as the change is slightly different from the original code.

facebook-github-bot · 2024-07-18T13:05:29Z

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

cipolleschi · 2024-07-19T08:36:41Z

Hi @zhongwuzw! A colleague suggested that we might just remove the callback.

Given this todo, we should be able to just start the surface after it is created.

Do you mind implementing the changes, please?

zhongwuzw · 2024-07-19T09:52:52Z

Hi @zhongwuzw! A colleague suggested that we might just remove the callback.
Given this todo, we should be able to just start the surface after it is created.
Do you mind implementing the changes, please?

@cipolleschi Hi, do you mean we can call the start surface on BufferedRuntimeExecutor?

cipolleschi · 2024-07-23T09:51:36Z

Yes, probably yes! :D

zhongwuzw · 2024-07-23T13:34:13Z

@cipolleschi I changed to bufferedruntime.

facebook-github-bot · 2024-07-30T09:41:29Z

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-08-02T20:52:42Z

@cipolleschi merged this pull request in a778979.

react-native-bot · 2024-08-02T20:52:44Z

This pull request was successfully merged by @zhongwuzw in a778979

^{When will my fix make it into a release? | How to file a pick request?}

migueldaipre · 2024-08-05T12:51:53Z


 for (RCTFabricSurface *surface in [self _getAttachedSurfaces]) {
 [surface resetWithSurfacePresenter:self.surfacePresenter];
+ [_instance callFunctionOnBufferedRumtimeExecutor:[surface](facebook::jsi::Runtime &_) { [surface start]; }];


Just a small typo "rumtime"

Thanks, good catch, I would fix it in my next PR. :)

Summary: I introduced a typo in #45486 . Thanks migueldaipre for the catch-up. cc cipolleschi ## Changelog: [IOS] [FIXED] - Fixes typo of function callFunctionOnBufferedRumtimeExecutor Pull Request resolved: #45902 Test Plan: Just a typo. Reviewed By: cipolleschi Differential Revision: D60775511 Pulled By: arushikesarwani94 fbshipit-source-id: da781ea5ecf2e0a15e5419430240e10194043b1b

[iOS] Fixes main thread stuck when reload in bridgeless mode
6e32378

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jul 17, 2024

cipolleschi requested changes Jul 17, 2024
View reviewed changes

Comment threadpackages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm Outdated

Add missed nil set
586c91f

zhongwuzw requested a review from cipolleschi July 18, 2024 03:19

cipolleschi approved these changes Jul 18, 2024
View reviewed changes

Call start surface on bufferedRuntime
ff282eb

zhongwuzw requested a review from cipolleschi July 30, 2024 07:53

facebook-github-bot closed this in a778979Aug 2, 2024

facebook-github-bot added the Merged This PR has been merged. label Aug 2, 2024

migueldaipre reviewed Aug 5, 2024
View reviewed changes

zhongwuzw mentioned this pull request Aug 5, 2024
[iOS] Fixes typo of function callFunctionOnBufferedRumtimeExecutor #45902
Closed

javache mentioned this pull request Dec 11, 2024
[iOS] Fabric: Fixes assert failure when surface stop before we start surface #48213
Closed

amorino mentioned this pull request Jul 19, 2026
Deadlock in ComponentDescriptorRegistry at app initalization #53128
Open

Navigation Menu

Uh oh!

[iOS] Fixes main thread stuck when reload in bridgeless mode - #45486

[iOS] Fixes main thread stuck when reload in bridgeless mode#45486
zhongwuzw wants to merge 3 commits into
react:mainfrom
zhongwuzw:features/fix_block_fabric

zhongwuzw commented Jul 17, 2024

Uh oh!

analysis-bot commented Jul 17, 2024•
edited
Loading

Uh oh!

cipolleschi left a comment

Uh oh!

Uh oh!

facebook-github-bot commented Jul 18, 2024

Uh oh!

cipolleschi commented Jul 19, 2024

Uh oh!

zhongwuzw commented Jul 19, 2024

Uh oh!

cipolleschi commented Jul 23, 2024

Uh oh!

zhongwuzw commented Jul 23, 2024

Uh oh!

facebook-github-bot commented Jul 30, 2024

Uh oh!

facebook-github-bot commented Aug 2, 2024

Uh oh!

react-native-bot commented Aug 2, 2024

Uh oh!

migueldaipreAug 5, 2024

Uh oh!

zhongwuzwAug 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

zhongwuzw commented Jul 17, 2024

Summary:

Changelog:

Test Plan:

Uh oh!

analysis-bot commented Jul 17, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-bot commented Jul 18, 2024

Uh oh!

cipolleschi commented Jul 19, 2024

Uh oh!

zhongwuzw commented Jul 19, 2024

Uh oh!

cipolleschi commented Jul 23, 2024

Uh oh!

zhongwuzw commented Jul 23, 2024

Uh oh!

facebook-github-bot commented Jul 30, 2024

Uh oh!

facebook-github-bot commented Aug 2, 2024

Uh oh!

react-native-bot commented Aug 2, 2024

Uh oh!

migueldaipreAug 5, 2024

Choose a reason for hiding this comment

Uh oh!

zhongwuzwAug 5, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

analysis-bot commented Jul 17, 2024•
edited
Loading