diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java index 8c520a71d..c56c70a50 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java @@ -3,10 +3,12 @@ package com.facebook.react.bridge; import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; import javax.inject.Provider; import java.util.concurrent.atomic.AtomicInteger; +import com.facebook.infer.annotation.Assertions; import com.facebook.proguard.annotations.DoNotStrip; import com.facebook.react.module.model.ReactModuleInfo; import com.facebook.systrace.Systrace; @@ -36,8 +38,13 @@ public class ModuleHolder { private final boolean mHasConstants; private @Nullable Provider mProvider; - private @Nullable NativeModule mModule; - private boolean mInitializeNeeded; + // Outside of the constructur, these should only be checked or set when synchronized on this + 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; public ModuleHolder(ReactModuleInfo moduleInfo, Provider provider) { mName = moduleInfo.name(); @@ -56,15 +63,28 @@ public class ModuleHolder { mModule = nativeModule; } - public synchronized void initialize() { - if (mModule != null) { - doInitialize(mModule); - } else { - mInitializeNeeded = true; + /* + * Checks if mModule has been created, and if so tries to initialize the module unless another + * thread is already doing the initialization. + * If mModule has not been created, records that initialization is needed + */ + /* package */ void markInitializable() { + boolean shouldInitializeNow = false; + NativeModule module = null; + synchronized (this) { + mInitializable = true; + if (mModule != null) { + Assertions.assertCondition(!mIsInitializing); + shouldInitializeNow = true; + module = mModule; + } + } + if (shouldInitializeNow) { + doInitialize(module); } } - public synchronized boolean isInitialized() { + /* pacakge */ synchronized boolean hasInstance() { return mModule != null; } @@ -88,11 +108,44 @@ public class ModuleHolder { } @DoNotStrip - public synchronized NativeModule getModule() { - if (mModule == null) { - mModule = create(); + public NativeModule getModule() { + NativeModule module; + boolean shouldCreate = false; + synchronized (this) { + if (mModule != null) { + return mModule; + // if mModule has not been set, and no one is creating it. Then this thread should call create + } else if (!mIsCreating) { + shouldCreate = true; + mIsCreating = true; + } else { + // Wait for mModule to be created by another thread + } + } + if (shouldCreate) { + module = create(); + // Once module is built (and initialized if markInitializable has been called), modify mModule + // And signal any waiting threads that it is acceptable to read the field now + synchronized (this) { + mIsCreating = false; + this.notifyAll(); + } + return module; + } else { + synchronized (this) { + // Block waiting for another thread to build mModule instance + // Since mIsCreating is true until after creation and instantiation (if needed), we wait + // until the module is ready to use. + while (mModule == null && mIsCreating) { + try { + this.wait(); + } catch (InterruptedException e) { + continue; + } + } + return Assertions.assertNotNull(mModule); + } } - return mModule; } private NativeModule create() { @@ -106,9 +159,15 @@ public class ModuleHolder { try { module = assertNotNull(mProvider).get(); mProvider = null; - if (mInitializeNeeded) { + boolean shouldInitializeNow = false; + synchronized(this) { + mModule = module; + if (mInitializable && !mIsInitializing) { + shouldInitializeNow = true; + } + } + if (shouldInitializeNow) { doInitialize(module); - mInitializeNeeded = false; } } finally { Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); @@ -123,7 +182,22 @@ public class ModuleHolder { .flush(); ReactMarker.logMarker(ReactMarkerConstants.INITIALIZE_MODULE_START, mName); try { - module.initialize(); + boolean shouldInitialize = false; + // Check to see if another thread is initializing the object, if not claim the responsibility + synchronized (this) { + if (mInitializable && !mIsInitializing) { + shouldInitialize = true; + mIsInitializing = true; + } + } + if (shouldInitialize) { + module.initialize(); + // Once finished, set flags accordingly, but we don't expect anyone to wait for this to finish + // So no need to notify other threads + synchronized (this) { + mIsInitializing = false; + } + } } finally { ReactMarker.logMarker(ReactMarkerConstants.INITIALIZE_MODULE_END); Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java index 2e2672766..9e2f5692e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java @@ -83,7 +83,7 @@ public class NativeModuleRegistry { "NativeModuleRegistry_notifyJSInstanceInitialized"); try { for (ModuleHolder module : mModules.values()) { - module.initialize(); + module.markInitializable(); } } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); @@ -93,7 +93,7 @@ public class NativeModuleRegistry { public void onBatchComplete() { for (ModuleHolder moduleHolder : mBatchCompleteListenerModules) { - if (moduleHolder.isInitialized()) { + if (moduleHolder.hasInstance()) { ((OnBatchCompleteListener) moduleHolder.getModule()).onBatchComplete(); } }