From 0394704452867b5beeeffb5aea589b24e268c594 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:30:59 +0000 Subject: [PATCH 1/5] Initial plan From a337cee9f8543ce581ece37cc165edc1610e21c8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 4 Sep 2025 08:38:47 +0000 Subject: [PATCH 2/5] Fix deadlock in ReactInstanceWin::DetachRootView by replacing sync call with async Co-authored-by: anupriya13 <54227869+anupriya13@users.noreply.github.com> --- .../ReactHost/ReactInstanceWin.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp b/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp index aefaaa3db63..c0c97337090 100644 --- a/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp +++ b/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp @@ -1514,8 +1514,15 @@ void ReactInstanceWin::DetachRootView(facebook::react::IReactRootView *rootView, CallJsFunction("AppRegistry", "unmountApplicationComponentAtRootTag", std::move(params)); } - // Give the JS thread time to finish executing - m_jsMessageThread.Load()->runOnQueueSync([]() {}); + // Schedule an async task on the JS thread to ensure previous operations complete + // This avoids the deadlock that can occur with runOnQueueSync when the JS thread + // is busy with garbage collection or other operations + if (auto jsMessageThread = m_jsMessageThread.Load()) { + jsMessageThread->runOnQueue([]() { + // This async operation ensures that any previously queued JS operations + // (like the unmount call above) have a chance to complete + }); + } } Mso::CntPtr MakeReactInstance( From 144551a713f22aa1d4724569b0ac2292e3dc423e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 4 Sep 2025 10:03:57 +0000 Subject: [PATCH 3/5] Add change file for prerelease and run formatting checks Co-authored-by: anupriya13 <54227869+anupriya13@users.noreply.github.com> --- change/1756980197-fix-deadlock.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/1756980197-fix-deadlock.json diff --git a/change/1756980197-fix-deadlock.json b/change/1756980197-fix-deadlock.json new file mode 100644 index 00000000000..866ed7a7c20 --- /dev/null +++ b/change/1756980197-fix-deadlock.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Fix deadlock in ReactInstanceWin::DetachRootView by replacing sync call with async", + "packageName": "react-native-windows", + "email": "copilot@github.com", + "dependentChangeType": "patch" +} From 4642723ccc17023a0d0356114e4bdcf34100a4f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Sep 2025 06:03:41 +0000 Subject: [PATCH 4/5] Remove sync call from DetachRootView to fix deadlock, add quirk option to re-enable if needed Co-authored-by: anupriya13 <54227869+anupriya13@users.noreply.github.com> --- change/remove-sync-detach-20250905060318.json | 7 +++++++ vnext/Microsoft.ReactNative/ReactHost/React.h | 6 ++++++ .../ReactHost/ReactInstanceWin.cpp | 14 ++++++-------- 3 files changed, 19 insertions(+), 8 deletions(-) create mode 100644 change/remove-sync-detach-20250905060318.json diff --git a/change/remove-sync-detach-20250905060318.json b/change/remove-sync-detach-20250905060318.json new file mode 100644 index 00000000000..b0f86bc73a0 --- /dev/null +++ b/change/remove-sync-detach-20250905060318.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Remove sync call from DetachRootView to fix deadlock, add quirk option to re-enable if needed", + "packageName": "@rnw-dependencies", + "email": "noreply@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/vnext/Microsoft.ReactNative/ReactHost/React.h b/vnext/Microsoft.ReactNative/ReactHost/React.h index dc7867f39b8..0c42b6b502f 100644 --- a/vnext/Microsoft.ReactNative/ReactHost/React.h +++ b/vnext/Microsoft.ReactNative/ReactHost/React.h @@ -251,6 +251,12 @@ struct ReactOptions { //! It is not safe to expose to Custom Function. Add this flag so we can turn it off for Custom Function. bool EnableNativePerformanceNow{true}; + //! QUIRK: Enable synchronous wait in DetachRootView for JS thread completion. + //! This flag re-enables the legacy runOnQueueSync call that can cause deadlocks + //! with Hermes GC. Only enable if removal of sync behavior breaks your application. + //! Default: false (sync call is disabled to prevent deadlocks) + bool EnableSyncDetachRootView{false}; + ReactDevOptions DeveloperSettings = {}; //! This controls the availability of various developer support functionality including diff --git a/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp b/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp index c0c97337090..930ace1d5e1 100644 --- a/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp +++ b/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp @@ -1514,15 +1514,13 @@ void ReactInstanceWin::DetachRootView(facebook::react::IReactRootView *rootView, CallJsFunction("AppRegistry", "unmountApplicationComponentAtRootTag", std::move(params)); } - // Schedule an async task on the JS thread to ensure previous operations complete - // This avoids the deadlock that can occur with runOnQueueSync when the JS thread - // is busy with garbage collection or other operations - if (auto jsMessageThread = m_jsMessageThread.Load()) { - jsMessageThread->runOnQueue([]() { - // This async operation ensures that any previously queued JS operations - // (like the unmount call above) have a chance to complete - }); + // QUIRK: Legacy sync behavior can be re-enabled via EnableSyncDetachRootView option + // The sync call was removed to prevent deadlocks with Hermes GC operations + if (m_options.EnableSyncDetachRootView) { + // Legacy behavior: wait for JS thread to finish executing (can cause deadlocks) + m_jsMessageThread.Load()->runOnQueueSync([]() {}); } + // Default behavior: no synchronization call to prevent deadlocks } Mso::CntPtr MakeReactInstance( From 84e081ba2b7ce0d8874cd68678643427dc9ee236 Mon Sep 17 00:00:00 2001 From: Anupriya Verma <54227869+anupriya13@users.noreply.github.com> Date: Tue, 9 Sep 2025 11:48:16 +0530 Subject: [PATCH 5/5] Address comment --- change/remove-sync-detach-20250905060318.json | 7 ------- vnext/Microsoft.ReactNative/QuirkSettings.cpp | 16 ++++++++++++++++ vnext/Microsoft.ReactNative/QuirkSettings.h | 3 +++ vnext/Microsoft.ReactNative/QuirkSettings.idl | 5 +++++ vnext/Microsoft.ReactNative/ReactHost/React.h | 6 ------ .../ReactHost/ReactInstanceWin.cpp | 4 +++- 6 files changed, 27 insertions(+), 14 deletions(-) delete mode 100644 change/remove-sync-detach-20250905060318.json diff --git a/change/remove-sync-detach-20250905060318.json b/change/remove-sync-detach-20250905060318.json deleted file mode 100644 index b0f86bc73a0..00000000000 --- a/change/remove-sync-detach-20250905060318.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "type": "prerelease", - "comment": "Remove sync call from DetachRootView to fix deadlock, add quirk option to re-enable if needed", - "packageName": "@rnw-dependencies", - "email": "noreply@microsoft.com", - "dependentChangeType": "patch" -} diff --git a/vnext/Microsoft.ReactNative/QuirkSettings.cpp b/vnext/Microsoft.ReactNative/QuirkSettings.cpp index 2ceca4b8a8a..1c817aa0f0a 100644 --- a/vnext/Microsoft.ReactNative/QuirkSettings.cpp +++ b/vnext/Microsoft.ReactNative/QuirkSettings.cpp @@ -77,6 +77,12 @@ winrt::Microsoft::ReactNative::ReactPropertyId UseRuntimeSchedulerProperty return propId; } +winrt::Microsoft::ReactNative::ReactPropertyId UseRunOnQueueSyncProperty() noexcept { + winrt::Microsoft::ReactNative::ReactPropertyId propId{L"ReactNative.QuirkSettings", L"UseRunOnQueueSync"}; + + return propId; +} + #pragma region IDL interface /*static*/ void QuirkSettings::SetMatchAndroidAndIOSStretchBehavior( @@ -121,6 +127,12 @@ winrt::Microsoft::ReactNative::ReactPropertyId UseRuntimeSchedulerProperty ReactPropertyBag(settings.Properties()).Set(UseRuntimeSchedulerProperty(), value); } +/*static*/ void QuirkSettings::SetUseRunOnQueueSync( + winrt::Microsoft::ReactNative::ReactInstanceSettings settings, + bool value) noexcept { + ReactPropertyBag(settings.Properties()).Set(UseRunOnQueueSyncProperty(), value); +} + #pragma endregion IDL interface /*static*/ bool QuirkSettings::GetMatchAndroidAndIOSStretchBehavior(ReactPropertyBag properties) noexcept { @@ -153,4 +165,8 @@ winrt::Microsoft::ReactNative::ReactPropertyId UseRuntimeSchedulerProperty return properties.Get(UseRuntimeSchedulerProperty()).value_or(true); } +/*static*/ bool QuirkSettings::GetUseRunOnQueueSync(ReactPropertyBag properties) noexcept { + return properties.Get(UseRunOnQueueSyncProperty()).value_or(false); +} + } // namespace winrt::Microsoft::ReactNative::implementation diff --git a/vnext/Microsoft.ReactNative/QuirkSettings.h b/vnext/Microsoft.ReactNative/QuirkSettings.h index 507c755c0d2..f19691a20c6 100644 --- a/vnext/Microsoft.ReactNative/QuirkSettings.h +++ b/vnext/Microsoft.ReactNative/QuirkSettings.h @@ -30,6 +30,7 @@ struct QuirkSettings : QuirkSettingsT { winrt::Microsoft::ReactNative::ReactPropertyBag properties) noexcept; static bool GetSuppressWindowFocusOnViewFocus(winrt::Microsoft::ReactNative::ReactPropertyBag properties) noexcept; static bool GetUseRuntimeScheduler(winrt::Microsoft::ReactNative::ReactPropertyBag properties) noexcept; + static bool GetUseRunOnQueueSync(winrt::Microsoft::ReactNative::ReactPropertyBag properties) noexcept; static bool GetEnableFabric(winrt::Microsoft::ReactNative::ReactPropertyBag properties) noexcept; @@ -65,6 +66,8 @@ struct QuirkSettings : QuirkSettingsT { winrt::Microsoft::ReactNative::ReactInstanceSettings settings, bool value) noexcept; + static void SetUseRunOnQueueSync(winrt::Microsoft::ReactNative::ReactInstanceSettings settings, bool value) noexcept; + #pragma endregion Public API - part of IDL interface }; diff --git a/vnext/Microsoft.ReactNative/QuirkSettings.idl b/vnext/Microsoft.ReactNative/QuirkSettings.idl index a4ad798aced..0770d5faa3f 100644 --- a/vnext/Microsoft.ReactNative/QuirkSettings.idl +++ b/vnext/Microsoft.ReactNative/QuirkSettings.idl @@ -64,6 +64,11 @@ namespace Microsoft.ReactNative "By default `react-native-windows` will use the new RuntimeScheduler." "Setting this to false will revert the behavior to previous scheduling logic.") static void SetUseRuntimeScheduler(ReactInstanceSettings settings, Boolean value); + + DOC_STRING( + "By default `react-native-windows` will not call sync JS call: runOnQueueSync." + "Setting this to true will revert the behavior to previous logic and might result in hang.") + static void SetUseRunOnQueueSync(ReactInstanceSettings settings, Boolean value); } } // namespace Microsoft.ReactNative diff --git a/vnext/Microsoft.ReactNative/ReactHost/React.h b/vnext/Microsoft.ReactNative/ReactHost/React.h index 0c42b6b502f..dc7867f39b8 100644 --- a/vnext/Microsoft.ReactNative/ReactHost/React.h +++ b/vnext/Microsoft.ReactNative/ReactHost/React.h @@ -251,12 +251,6 @@ struct ReactOptions { //! It is not safe to expose to Custom Function. Add this flag so we can turn it off for Custom Function. bool EnableNativePerformanceNow{true}; - //! QUIRK: Enable synchronous wait in DetachRootView for JS thread completion. - //! This flag re-enables the legacy runOnQueueSync call that can cause deadlocks - //! with Hermes GC. Only enable if removal of sync behavior breaks your application. - //! Default: false (sync call is disabled to prevent deadlocks) - bool EnableSyncDetachRootView{false}; - ReactDevOptions DeveloperSettings = {}; //! This controls the availability of various developer support functionality including diff --git a/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp b/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp index 930ace1d5e1..906364b8ab3 100644 --- a/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp +++ b/vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp @@ -1516,7 +1516,9 @@ void ReactInstanceWin::DetachRootView(facebook::react::IReactRootView *rootView, // QUIRK: Legacy sync behavior can be re-enabled via EnableSyncDetachRootView option // The sync call was removed to prevent deadlocks with Hermes GC operations - if (m_options.EnableSyncDetachRootView) { + bool useSyncCall = winrt::Microsoft::ReactNative::implementation::QuirkSettings::GetUseRunOnQueueSync( + winrt::Microsoft::ReactNative::ReactPropertyBag(m_reactContext->Properties())); + if (useSyncCall) { // Legacy behavior: wait for JS thread to finish executing (can cause deadlocks) m_jsMessageThread.Load()->runOnQueueSync([]() {}); }