ammarahm-ed / react-native-mmkv-storage

An ultra fast (0.0002s read/write), small & encrypted mobile key-value storage framework for React Native written in C++ using JSI
https://rnmmkv.now.sh
MIT License
1.6k stars 111 forks source link

Use RCTBridgeModule's bridge property instead of [RCTBridge currentBridge] #249

Closed cltnschlosser closed 2 years ago

cltnschlosser commented 2 years ago

From the docs of RCTBridge:

/**
 * The last current active bridge instance. This is set automatically whenever
 * the bridge is accessed. It can be useful for static functions or singletons
 * that need to access the bridge for purposes such as logging, but should not
 * be relied upon to return any particular instance, due to race conditions.
 */
+ (instancetype)currentBridge
{
  return RCTCurrentBridgeInstance;
}

What I've found is there exists a race condition where a JS reload triggered very early could potentially result in MMKV install failing in the initial JS context. Much of this is due to the bridge invalidation being asynchronous. It looks something like this:

*Very precisely placed steps to trigger this race condition

Realistically we don't care that the MMKV install in bridge 1 failed, except for the fact that it will throw an error (which can crash the app). Using _bridge we have access the correct bridge, even if it's invalidation has started. The install work in the bridge 1 context may be unnecessary and will be later thrown away when the invalidation is finished, but at least it doesn't error. Later MMKV install is called in bridge 2's context and this is what will be used going forward.

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
rnmmkv ✅ Ready (Inspect) Visit Preview May 9, 2022 at 7:29PM (UTC)
ammarahm-ed commented 2 years ago

Thanks for debugging this and sending a fix! Looks good.