Skip to content

Commit

Permalink
Make NativeModules immediately initializable
Browse files Browse the repository at this point in the history
Summary:
## Problem:
NativeModules can only be initialized after we mark them initializable on the NativeModules thread. This work is scheduled in ReactInstanceManager.setupReactContext(), after we schedule the execution of the JS bundle on the JavaScript thread in ReactInstanceManager.createReactContext():

https://www.internalfb.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java?commit=9b58f58b8eb12281567b8d24d6d000e868e61a1f&lines=1256-1257%2C1331%2C1334-1335%2C1333-1334

So, if the timings don't work out, the JavaScript thread could start executing the JS bundle before the NativeModule thread makes all NativeModules initializable. In this case, those NativeModule will just appear to be null in C++/JavaScript.

## Fix
This diff makes all NativeModules initializable immediately after  their ModuleHolder is created. ModuleHolder.markInitializable() simply initializes initializes modules that were eagerly created.

Changelog: [Android][Fixed] - Make NativeModules immediately initializable

Reviewed By: JoshuaGross

Differential Revision: D26233372

fbshipit-source-id: a9223ff093da5b80781169be88e6ec9516c7a29b
  • Loading branch information
RSNara authored and facebook-github-bot committed Feb 4, 2021
1 parent f66dcab commit 2bf866e
Showing 1 changed file with 2 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ public class ModuleHolder {
private @Nullable @GuardedBy("this") NativeModule mModule;

// These are used to communicate phases of creation and initialization across threads
private @GuardedBy("this") boolean mInitializable;
private @GuardedBy("this") boolean mIsCreating;
private @GuardedBy("this") boolean mIsInitializing;

Expand Down Expand Up @@ -89,7 +88,6 @@ public ModuleHolder(NativeModule nativeModule) {
boolean shouldInitializeNow = false;
NativeModule module = null;
synchronized (this) {
mInitializable = true;
if (mModule != null) {
Assertions.assertCondition(!mIsInitializing);
shouldInitializeNow = true;
Expand Down Expand Up @@ -193,7 +191,7 @@ private NativeModule create() {
boolean shouldInitializeNow = false;
synchronized (this) {
mModule = module;
if (mInitializable && !mIsInitializing) {
if (!mIsInitializing) {
shouldInitializeNow = true;
}
}
Expand Down Expand Up @@ -227,7 +225,7 @@ private void doInitialize(NativeModule module) {
boolean shouldInitialize = false;
// Check to see if another thread is initializing the object, if not claim the responsibility
synchronized (this) {
if (mInitializable && !mIsInitializing) {
if (!mIsInitializing) {
shouldInitialize = true;
mIsInitializing = true;
}
Expand Down

0 comments on commit 2bf866e

Please sign in to comment.