Maximize/Restore window crashes app

[asklar]

launch playground or E2ETest app - i think you have to do it without starting the bundler
maximize, restore
App crashes with the stack below.

I believe this is because connecting to the bundler sets up a callback in facebook code. In the crashing case the callback never gets set by RN core, but we still try to call the callback...

 # ChildEBP RetAddr  
00 1438dc4c 18ccaec7 Microsoft_ReactNative_17e10000!facebook::react::Instance::callJSFunction+0x41 [C:\rnw\vnext\build\x86\Debug\ReactCommon\react-native-patched\ReactCommon\cxxreact\Instance.cpp @ 189] 
01 1438dd6c 18cd874e Microsoft_ReactNative_17e10000!Mso::React::ReactInstanceWin::CallJsFunction+0xf7 [C:\rnw\vnext\Microsoft.ReactNative\ReactHost\ReactInstanceWin.cpp @ 639] 
02 1438de8c 187b9a29 Microsoft_ReactNative_17e10000!react::uwp::UwpReactInstanceProxy::CallJsFunction+0xee [C:\rnw\vnext\Microsoft.ReactNative\ReactHost\UwpReactInstanceProxy.cpp @ 68] 
03 1438e070 187b2fb1 Microsoft_ReactNative_17e10000!react::uwp::DeviceInfo::fireEvent+0x159 [C:\rnw\vnext\ReactUWP\Modules\DeviceInfoModule.cpp @ 58] 
04 1438e150 187b8871 Microsoft_ReactNative_17e10000!<lambda_5f54b12040f03c6eccc4342c7a4ff06a>::operator()<winrt::Windows::UI::Core::CoreWindow const &,winrt::Windows::UI::Core::WindowSizeChangedEventArgs const &>+0x41 [C:\rnw\vnext\ReactUWP\Modules\DeviceInfoModule.cpp @ 29] 
05 1438e25c 7ab07556 Microsoft_ReactNative_17e10000!winrt::impl::delegate<winrt::Windows::Foundation::TypedEventHandler<winrt::Windows::UI::Core::CoreWindow,winrt::Windows::UI::Core::WindowSizeChangedEventArgs>,<lambda_5f54b12040f03c6eccc4342c7a4ff06a> >::Invoke+0x71 [C:\rnw\vnext\build\x86\Debug\Microsoft.ReactNative\Generated Files\winrt\Windows.Foundation.h @ 893] 
06 (Inline) -------- Windows_UI!Microsoft::WRL::EventSource<Windows::Foundation::ITypedEventHandler<Windows::UI::Core::CoreWindow *,Windows::UI::Core::WindowSizeChangedEventArgs *>,Microsoft::WRL::InvokeModeOptions<-2> >::InvokeAll::__l2::<lambda_f2339b31eef01df78e3af58019e05583>::operator()+0x1d [onecore\external\sdk\inc\wrl\event.h @ 954] 
07 1438e298 7ab06f67 Windows_UI!Microsoft::WRL::InvokeTraits<-2>::InvokeDelegates<<lambda_f2339b31eef01df78e3af58019e05583>,Windows::Foundation::ITypedEventHandler<Windows::UI::Core::CoreWindow *,Windows::UI::Core::WindowSizeChangedEventArgs *> >+0x4e [onecore\internal\sdk\inc\wrl\internalevent.h @ 119] 
08 1438e2c8 7aae7406 Windows_UI!Microsoft::WRL::EventSource<Windows::Foundation::ITypedEventHandler<Windows::UI::Core::CoreWindow *,Windows::UI::Core::WindowSizeChangedEventArgs *>,Microsoft::WRL::InvokeModeOptions<-2> >::DoInvoke<<lambda_f2339b31eef01df78e3af58019e05583> >+0x67 [onecore\external\sdk\inc\wrl\event.h @ 946] 
09 (Inline) -------- Windows_UI!Microsoft::WRL::EventSource<Windows::Foundation::ITypedEventHandler<Windows::UI::Core::CoreWindow *,Windows::UI::Core::WindowSizeChangedEventArgs *>,Microsoft::WRL::InvokeModeOptions<-2> >::InvokeAll+0x19 [onecore\external\sdk\inc\wrl\event.h @ 954] 
0a 1438e324 7aae730d Windows_UI!Windows::UI::Core::WindowServer::SendWindowSizeEvent+0xe6 [onecoreuap\windows\advcore\winrt\onecoreiwindow\corewindow\common\windowserver.cpp @ 2911] 
0b 1438e33c 7aae7952 Windows_UI!Windows::UI::Core::WindowServer::OnWindowSizeEvent+0x2e [onecoreuap\windows\advcore\winrt\onecoreiwindow\corewindow\common\windowserver.cpp @ 2776] 
0c 1438e3c8 75c47a83 Windows_UI!Windows::UI::Core::WindowServer::WndProc+0x1b2 [onecoreuap\windows\advcore\winrt\onecoreiwindow\corewindow\common\windowserver.cpp @ 7873] 


00 1438dc4c 18ccaec7 Microsoft_ReactNative_17e10000!facebook::react::Instance::callJSFunction+0x41 [C:\rnw\vnext\build\x86\Debug\ReactCommon\react-native-patched\ReactCommon\cxxreact\Instance.cpp @ 189] 
0:009> dx this
this                 : 0x2fba0f8c [Type: facebook::react::Instance *]
    [+0x000] callback_        : empty [Type: std::shared_ptr<facebook::react::InstanceCallback>]
    [+0x008] nativeToJsBridge_ : empty [Type: std::unique_ptr<facebook::react::NativeToJsBridge,std::default_delete<facebook::react::NativeToJsBridge> >]
    [+0x00c] moduleRegistry_  : empty [Type: std::shared_ptr<facebook::react::ModuleRegistry>]
    [+0x014] m_syncMutex      [Type: std::mutex]
    [+0x044] m_syncCV         [Type: std::condition_variable]
    [+0x06c] m_syncReady      : false [Type: bool]

[asklar]

The short of it is – when we start an app without the bundler, we correctly go into a HasError state and show the redbox.
However we get a call to OnReactInstanceLoaded from the destructor of LoadedCallbackGuard – this moves us to the Loaded state. Then when we get callbacks from native modules, we actually try calling into the JS side, but the bridge hasn’t been set up so we crash.
I tried seeing what would happen if we don’t go into the Loaded state and we don’t crash any longer. However the Reload functionality is broken (we don’t attempt to reload the bundle anymore if we’re in the HasError state)

---

The reason we run into this is because of the following sequence of events

• app starts and tries to load bundle but the bundler is not running. Instance moves to m_state = ReactInstanceState::HasError
• Asynchronously we get called ReactInstanceWin::OnReactInstanceLoaded which moves us to m_state = ReactInstanceState::Loaded
• From then on, when there is an event like changing app state between restored/minimized/maximized, we see that the app is Loaded so we go forward trying to call the callback. However the callback was never set up because we early return in the catch block in InstanceImpl::InstanceImpl:

    try {
      auto jseFunc = m_devManager->LoadJavaScriptInProxyMode(*m_devSettings);

      if ((jseFunc == nullptr) || m_devManager->HasException()) {
        m_devSettings->errorCallback("Failed to create JavaScript Executor.");
        return;
      }

      jsef = std::make_shared<WebSocketJSExecutorFactory>(std::move(jseFunc));
    } catch (std::exception &e) {
      m_devSettings->errorCallback(e.what());
      return;
    }

So it sounds like whatever is causing us to go into the Loaded state is wrong and we should avoid this if we're in HasError. However I tried bypassing transitioning into Loaded from HasError and I found that this breaks because things like the Reload button stop working if we're in this state - I assume something must be expecting to us be in a Loaded state.

@vmoroz The code that's doing the unloading/loading is kind of hard to follow as it's going into AsyncActionQueue and some Mso futures thing that I don't fully understand... the queue seems to get the requests to unload and reload, but nothing is running that queue... are we just missing some message pump to drain that queue? who's supposed to be doing this and why does it work when the instance is in a loaded state?

[vmoroz]

@asklar , the AsyncActionQueue is driven on it own. Next action is executed when the previous reports completion. We just need to consider the error state as the load operation completion state to enable the next actions to be executed. I will have a look at it.

[chrisglein]

May only impact development scenarios, but still pretty bad to crash on maximize :)

[asklar]

Spoke to @vmoroz offline and explained the issue in detail, it's a bug on the instance management side, he'll take a look.

[vmoroz]

Yes, currently we have inconsistency in how we manage the React instance loading states. Some code assumes that terminal state is Loaded, while others consider Loaded and HasError to be both terminal states. We must fix the code to always use Loaded and HasError as terminal states.