Get dev mode reload working (#112)

This change includes a bunch of fixes for React Native dev mode reload (which reloads the JS engine).

- Update to the latest BabylonNative submodule which includes a fix to actually delete the napi instance on Napi::Detach.
- Set the current NativeEngine instance on the _native object so we can get it on the native side and manually dispose it (since during a reload the EngineHook's cleanup function will not be called, but we still need to release these resources).
- Have the native interop code grab the current NativeEngine instance off of _native during teardown and call dispose on it.
- Prevent the JS dispatcher from actually doing anything if we are in the process of shutting down the JS engine.

These changes are a little icky and there is a fair bit of duplication across Android and iOS. I have a bigger cleanup planned for this code that will consolidate a lot of the Android and iOS code, but that will happen later.

Fixes #84

Author: ryantrem

Modules/@babylonjs/react-native/BabylonModule.ts

@@ -1,6 +1,31 @@
 import { NativeModules } from 'react-native';
+import { NativeEngine } from '@babylonjs/core';
 
-export const BabylonModule: {
+// This global object is part of Babylon Native.
+declare const _native: {
+    graphicsInitializationPromise: Promise<void>;
+    engineInstance: NativeEngine;
+}
+
+const NativeBabylonModule: {
     initialize(): Promise<boolean>;
     whenInitialized(): Promise<boolean>;
-} = NativeModules.BabylonModule;
+} = NativeModules.BabylonModule;
+
+export const BabylonModule = {
+    initialize: async () => {
+        const initialized = await NativeBabylonModule.initialize();
+        if (initialized) {
+            await _native.graphicsInitializationPromise;
+        }
+        return initialized;
+    },
+
+    whenInitialized: NativeBabylonModule.whenInitialized,
+
+    createEngine: () => {
+        const engine = new NativeEngine();
+        _native.engineInstance = engine;
+        return engine;
+    }
+};

Modules/@babylonjs/react-native/EngineHook.ts

@@ -62,11 +62,6 @@ class DOMException {
 declare const global: any;
 global.atob = base64.decode;
 
-// This global object is part of Babylon Native.
-declare const _native: {
-    graphicsInitializationPromise: Promise<void>;
-}
-
 export function useEngine(): Engine | undefined {
     const [engine, setEngine] = useState<Engine>();
 
@@ -77,8 +72,7 @@ export function useEngine(): Engine | undefined {
         (async () => {
             if (await BabylonModule.initialize() && !disposed)
             {
-                await _native.graphicsInitializationPromise;
-                engine = new NativeEngine();
+                engine = BabylonModule.createEngine();
                 setEngine(engine);
             }
         })();

Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp

@@ -33,6 +33,8 @@ namespace Babylon
         {
             __android_log_print(ANDROID_LOG_VERBOSE, "BabylonNative", "%s", str);
         }
+
+        bool isShuttingDown{false};
     }
 
     class Native final
@@ -42,7 +44,9 @@ namespace Babylon
         Native(jsi::Runtime& jsiRuntime, std::shared_ptr<react::CallInvoker> callInvoker, ANativeWindow* windowPtr)
             : m_env{ Napi::Attach<jsi::Runtime&>(jsiRuntime) }
         {
-            m_runtime = &JsRuntime::CreateForJavaScript(m_env, CreateJsRuntimeDispatcher(m_env, jsiRuntime, callInvoker));
+            isShuttingDown = false;
+
+            m_runtime = &JsRuntime::CreateForJavaScript(m_env, CreateJsRuntimeDispatcher(m_env, jsiRuntime, std::move(callInvoker), isShuttingDown));
 
             auto width = static_cast<size_t>(ANativeWindow_getWidth(windowPtr));
             auto height = static_cast<size_t>(ANativeWindow_getHeight(windowPtr));
@@ -61,10 +65,18 @@ namespace Babylon
             m_nativeInput = &Babylon::Plugins::NativeInput::CreateForJavaScript(m_env);
         }
 
+        // NOTE: This only happens when the JS engine is shutting down (other than when the app exits, this only
+        //       happens during a dev mode reload). In this case, EngineHook.ts won't call NativeEngine.dispose,
+        //       so we need to manually do it here to properly clean up these resources.
         ~Native()
         {
-            // TODO: Figure out why this causes the app to crash
-            //Napi::Detach(m_env);
+            auto native = JsRuntime::NativeObject::GetFromJavaScript(m_env);
+            auto engine = native.Get("engineInstance").As<Napi::Object>();
+            auto dispose = engine.Get("dispose").As<Napi::Function>();
+            dispose.Call(engine, {});
+            isShuttingDown = true;
+
+            Napi::Detach(m_env);
         }
 
         void Refresh(ANativeWindow* windowPtr)

Modules/@babylonjs/react-native/android/src/main/java/com/babylonreactnative/BabylonModule.java

@@ -22,9 +22,10 @@ public String getName() {
         return "BabylonModule";
     }
 
+    // NOTE: This happens during dev mode reload, when the JS engine is being shutdown and restarted.
     @Override
     public void onCatalystInstanceDestroy() {
-        new Handler(Looper.getMainLooper()).post(BabylonNativeInterop::deinitialize);
+        this.getReactApplicationContext().runOnJSQueueThread(BabylonNativeInterop::deinitialize);
     }
 
     @ReactMethod

Modules/@babylonjs/react-native/android/src/main/java/com/babylonreactnative/BabylonNativeInterop.java

@@ -132,7 +132,7 @@ static CompletionStage<Long> whenInitialized(ReactContext reactContext) {
         return BabylonNativeInterop.getOrCreateFuture(reactContext);
     }
 
-    // Must be called from the Android UI thread
+    // Must be called from the JavaScript thread
     static void deinitialize() {
         BabylonNativeInterop.destroyOldNativeInstances(null);
     }

Modules/@babylonjs/react-native/ios/BabylonNative.cpp

@@ -24,6 +24,11 @@ namespace Babylon
 {
     using namespace facebook;
 
+    namespace
+    {
+        bool isShuttingDown{false};
+    }
+
     class Native::Impl
     {
     public:
@@ -32,24 +37,28 @@ namespace Babylon
             , jsCallInvoker{ callInvoker }
         {
         }
+        
+        ~Impl()
+        {
+            Napi::Detach(env);
+        }
 
         Napi::Env env;
         std::shared_ptr<facebook::react::CallInvoker> jsCallInvoker;
-        std::unique_ptr<Graphics> m_graphics{};
+        std::unique_ptr<Graphics> graphics{};
         JsRuntime* runtime{};
         Plugins::NativeInput* nativeInput{};
     };
 
     Native::Native(facebook::jsi::Runtime& jsiRuntime, std::shared_ptr<facebook::react::CallInvoker> callInvoker, void* windowPtr, size_t width, size_t height)
         : m_impl{ std::make_unique<Native::Impl>(jsiRuntime, callInvoker) }
     {
-        dispatch_sync(dispatch_get_main_queue(), ^{
-            m_impl->m_graphics = Graphics::CreateGraphics(reinterpret_cast<void*>(windowPtr), width, height);
-        });
+        isShuttingDown = false;
+        m_impl->graphics = Graphics::CreateGraphics(reinterpret_cast<void*>(windowPtr), width, height);
 
-        m_impl->runtime = &JsRuntime::CreateForJavaScript(m_impl->env, CreateJsRuntimeDispatcher(m_impl->env, jsiRuntime, callInvoker));
-        
-        m_impl->m_graphics->AddToJavaScript(m_impl->env);
+        m_impl->runtime = &JsRuntime::CreateForJavaScript(m_impl->env, CreateJsRuntimeDispatcher(m_impl->env, jsiRuntime, std::move(callInvoker), isShuttingDown));
+
+        m_impl->graphics->AddToJavaScript(m_impl->env);
 
         Polyfills::Window::Initialize(m_impl->env);
         // NOTE: React Native's XMLHttpRequest is slow and allocates a lot of memory. This does not override
@@ -62,19 +71,27 @@ namespace Babylon
         m_impl->nativeInput = &Babylon::Plugins::NativeInput::CreateForJavaScript(m_impl->env);
     }
 
+    // NOTE: This only happens when the JS engine is shutting down (other than when the app exits, this only
+    //       happens during a dev mode reload). In this case, EngineHook.ts won't call NativeEngine.dispose,
+    //       so we need to manually do it here to properly clean up these resources.
     Native::~Native()
     {
+        auto native = JsRuntime::NativeObject::GetFromJavaScript(m_impl->env);
+        auto engine = native.Get("engineInstance").As<Napi::Object>();
+        auto dispose = engine.Get("dispose").As<Napi::Function>();
+        dispose.Call(engine, {});
+        isShuttingDown = true;
     }
 
     void Native::Refresh(void* windowPtr, size_t width, size_t height)
     {
-        m_impl->m_graphics->UpdateWindow<void*>(windowPtr);
-        m_impl->m_graphics->UpdateSize(width, height);
+        m_impl->graphics->UpdateWindow<void*>(windowPtr);
+        m_impl->graphics->UpdateSize(width, height);
     }
 
     void Native::Resize(size_t width, size_t height)
     {
-        m_impl->m_graphics->UpdateSize(width, height);
+        m_impl->graphics->UpdateSize(width, height);
     }
 
     void Native::SetPointerButtonState(uint32_t pointerId, uint32_t buttonId, bool isDown, uint32_t x, uint32_t y)

Modules/@babylonjs/react-native/ios/BabylonNativeInterop.mm

@@ -126,7 +126,16 @@ + (void)setCurrentNativeInstance:(RCTBridge*)bridge mtkView:(MTKView*)mtkView wi
     {
         const std::lock_guard<std::mutex> lock(mapMutex);
 
-        currentBridge = bridge;
+        if (bridge != currentBridge) {
+            if (currentBridge == nil || currentBridge.parentBridge != bridge.parentBridge) {
+                [[NSNotificationCenter defaultCenter] addObserver:self
+                    selector:@selector(onBridgeWillInvalidate:)
+                    name:RCTBridgeWillInvalidateModulesNotification
+                    object:bridge.parentBridge];
+            }
+
+            currentBridge = bridge;
+        }
 
         currentNativeInstance.reset();
 
@@ -146,4 +155,10 @@ + (void)setCurrentNativeInstance:(RCTBridge*)bridge mtkView:(MTKView*)mtkView wi
     }
 }
 
+// NOTE: This happens during dev mode reload, when the JS engine is being shutdown and restarted.
++ (void)onBridgeWillInvalidate:(NSNotification*)notification
+{
+    currentNativeInstance.reset();
+}
+
 @end

Modules/@babylonjs/react-native/shared/DispatchFunction.h

@@ -10,9 +10,9 @@ namespace Babylon
     using namespace facebook;
 
     // Creates a JsRuntime::DispatchFunctionT that integrates with the React Native execution environment.
-    inline JsRuntime::DispatchFunctionT CreateJsRuntimeDispatcher(Napi::Env env, jsi::Runtime& jsiRuntime, std::shared_ptr<react::CallInvoker> callInvoker)
+    inline JsRuntime::DispatchFunctionT CreateJsRuntimeDispatcher(Napi::Env env, jsi::Runtime& jsiRuntime, std::shared_ptr<react::CallInvoker> callInvoker, const bool& isShuttingDown)
     {
-        return [env, &jsiRuntime, callInvoker](std::function<void(Napi::Env)> func)
+        return [env, &jsiRuntime, callInvoker, &isShuttingDown](std::function<void(Napi::Env)> func)
         {
             // Ideally we would just use CallInvoker::invokeAsync directly, but currently it does not seem to integrate well with the React Native logbox.
             // To work around this, we wrap all functions in a try/catch, and when there is an exception, we do the following:
@@ -23,11 +23,15 @@ namespace Babylon
             // 1. setImmediate queues the callback, and that queue is drained immediately following the invocation of the function passed to CallInvoker::invokeAsync.
             // 2. The immediates queue is drained as part of the class bridge, which knows how to display the logbox for unhandled exceptions.
             // In the future, CallInvoker::invokeAsync likely will properly integrate with logbox, at which point we can remove the try/catch and just call func directly.
-            callInvoker->invokeAsync([env, &jsiRuntime, func{std::move(func)}]
+            callInvoker->invokeAsync([env, &jsiRuntime, func{std::move(func)}, &isShuttingDown]
             {
                 try
                 {
-                    func(env);
+                    // If JS engine shutdown is in progress, don't dispatch any new work.
+                    if (!isShuttingDown)
+                    {
+                        func(env);
+                    }
                 }
                 catch (...)
                 {
@@ -43,4 +47,4 @@ namespace Babylon
             });
         };
     }
-}
+}