fix: prevent EXC_BAD_ACCESS in Promise destructor after runtime teardown

[tbrushwyler]

Summary

Fixes a crash (EXC_BAD_ACCESS / null pointer dereference in jsi::Pointer::~Pointer()) that occurs when a JsiPromises::Promise is destroyed after the JSI runtime has been torn down.

This commonly happens when async operations like Skia.Data.fromURI() (used by useImage, useFont, useSVG, useImageAsTexture) complete after the React Native bridge has shut down — for example, when a user navigates away from a screen while images are still loading, or when the app reloads.

Stack trace

0  jsi.h:465        facebook::jsi::Pointer::~Pointer()
1  jsi.h:775        facebook::jsi::Object::~Object()
2  jsi.h:1120       facebook::jsi::Function::~Function()
4  JsiPromises.h:32 RNJsi::JsiPromises::Promise::~Promise()
10 JsiSkDataFactory.h:37 RNSkia::JsiSkDataFactory::fromURI(...)::lambda

Root cause

Promise holds jsi::Function members (resolve_, reject_) whose destructors call jsi::Pointer::~Pointer(), which accesses the JSI runtime to release the underlying PointerValue. When the runtime has already been torn down, this dereferences an invalid pointer.

Fix

Add an explicit destructor that std::moves the jsi::Function members to the heap before implicit member destruction runs. After the move, the member's internal ptr_ is null (per jsi::Pointer's move constructor: other.ptr_ = nullptr), so implicit destruction safely skips the runtime access. The heap copies are intentionally leaked (~32 bytes per Promise) since there is no safe way to release JSI pointers without a valid runtime.

Test plan

• yarn tsc passes
• yarn lint passes
• yarn test passes (for @shopify/react-native-skia)
• Navigate to a screen that triggers async Skia image/font/SVG loading (e.g. via useImage)
• While assets are loading, navigate away or trigger a bridge teardown (reload, bundle swap)
• Verify no crash occurs
• Verify normal image loading still works when staying on the screen

[tbrushwyler]

I have signed the CLA!

[wcandillon]

This would cause a memory leak which maybe is ok because this situation can only happen in DEV mode or during OTA. Still I think there is a proper way to do it, I can give it a shot if you want.
It also only protect the destructor not the reject/resolve itself.

[tbrushwyler]

This would cause a memory leak which maybe is ok because this situation can only happen in DEV mode or during OTA. Still I think there is a proper way to do it, I can give it a shot if you want. It also only protect the destructor not the reject/resolve itself.

Good catch - I've amended the commit to fix the memory leak with better cleanup and to make resolve() & reject() no-ops if the runtime is gone. Feel free to adjust if you see a better way to do it.

[wcandillon]

Thank you for looking into this and I like it a lot. Added a small comment for onRuntimeDestroyed and ~Promise.

I'm always struggling to think through these use-cases. One approach I am thinking about (not sure if it's good or not) would be to use jsi::Runtime* as a key in the RuntimeLifecycleMonitor so we can safely check if it's there or not.

[wcandillon]

Here we do not check if the destroyed runtime is the one we hold a reference to? We should add a check there no?

[tbrushwyler]

Good idea - added a check: 5d1d3bf

[wcandillon]

That's look elegant to me. And it think this is safe if we set runtimeDestroyed_ = true; if the runtime ref match the one Promise hold.

[wcandillon]

@tbrushwyler do you have tips on how to reproduce the issue? I run a jsi promise that settles if the runtime gets destroyed:

  // Debug method to reproduce race condition: tries to resolve promise when
  // runtime is destroyed
  JSI_HOST_FUNCTION(runLongLivedPromise) {
    return RNJsi::JsiPromises::createPromiseAsJSIValue(
        runtime,
        [](jsi::Runtime &runtime,
           std::shared_ptr<RNJsi::JsiPromises::Promise> promise) -> void {
          // Create a listener that will try to resolve when runtime is
          // destroyed
          class CrashListener : public RNJsi::RuntimeLifecycleListener {
          public:
            CrashListener(std::shared_ptr<RNJsi::JsiPromises::Promise> p)
                : _promise(std::move(p)) {}

            void onRuntimeDestroyed(jsi::Runtime *rt) override {
              // This should crash - trying to resolve on a destroyed runtime
              _promise->resolve(jsi::Value::undefined());
              delete this;
            }

          private:
            std::shared_ptr<RNJsi::JsiPromises::Promise> _promise;
          };

          auto *listener = new CrashListener(promise);
          RNJsi::RuntimeLifecycleMonitor::addListener(runtime, listener);
        });
  }

But doing hot reload while running this function doesn't seem to cause a crash. In the debugger I have a breakpoint on _promise->resolve that gets hit when doing hot reload but I am not having any crash.

[tbrushwyler]

@wcandillon here's the stack trace for the crash that this change fixes:

EXC_BAD_ACCESS: Attempted to dereference null pointer.

0  facebook::jsi::Pointer::~Pointer() (jsi.h:465:13)
1  facebook::jsi::Object::~Object() (jsi.h:775:18)
2  facebook::jsi::Function::~Function() (jsi.h:1120:18)
3  facebook::jsi::Function::~Function() (jsi.h:1120:18)
4  RNJsi::JsiPromises::Promise::~Promise() (JsiPromises.h:32:10)
5  RNJsi::JsiPromises::Promise::~Promise() (JsiPromises.h:32:10)
6  std::shared_ptr<RNJsi::JsiPromises::Promise>::~shared_ptr() (shared_ptr.h:558:17)
7  std::shared_ptr<RNJsi::JsiPromises::Promise>::~shared_ptr() (shared_ptr.h:556:39)
8  RNSkia::JsiSkDataFactory::fromURI(...)::lambda::~() (JsiSkDataFactory.h:37:48)

This is an intermittent issue that we see, exacerbated by the way that we're creating individual activities/view controllers that host a RN bundle. We see the crash when we navigate away from one of these RN views before its Skia promises have all completed.

[wcandillon]

I will still try to reproduce the issue if I can. Now on your side this patch is fixing the issue for you? Do you have a small example project to test it?

[tbrushwyler]

@wcandillon I do not have a minimal repro, but this does fix the issue for me.