Control concurrent calls into TMMDelegate from TMM

Summary:
In D20659799, I improved `TurboModuleManager.getModule(moduleName)` thread-safety by ensuring that if two threads race to require the same NativeModule, only one thread creates the NativeModule, while the other one waits until it's created.

## The problem:
What I failed to realize was that when two threads race to require two different NativeModules, we can get concurrent calls into `TurboModuleManagerDelegate.getModule(moduleName)`, and `TurboModuleManagerDelegate.getLegacyCxxModule(moduleName)`, which don't have any thread-safe guarantees.

## The fix
`TurboModuleManagerDelegate` is supposed to be an input to the TurboModule system. So, rather than expecting that all TurboModuleManagerDelegates are thread-safe, which might be a reasonable ask (see T65532092), this diff has `TurboModuleManager` acquire the delegate's lock before calling into it. This ensures that we don't get concurrent access into the delegate, which could be reading from, or writing to, some data structure in these method calls. (This was the case with `ReactPackageTurboModuleManagerDelegate`, which is what Fb4a and Workplace use under the hood).

Changelog:
[Android][Fixed] - Control concurrent calls into TMMDelegate from TurboModuleManager

Reviewed By: mdvacca

Differential Revision: D21025965

fbshipit-source-id: d22c4abfe87f9e534717a06f186dde87d3cd24df
This commit is contained in:
Ramanpreet Nara
2020-04-14 18:27:16 -07:00
committed by Facebook GitHub Bot
parent 36688d35e1
commit bc99a32e4d
@@ -32,7 +32,7 @@ import java.util.*;
*/
public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
private static volatile boolean sIsSoLibraryLoaded;
private final List<String> mEagerInitModuleNames;
private final List<String> mEagerInitModuleNames = new ArrayList<>();
private final TurboModuleProvider mJavaModuleProvider;
private final TurboModuleProvider mCxxModuleProvider;
@@ -64,8 +64,11 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
delegate);
installJSIBindings();
mEagerInitModuleNames =
delegate == null ? new ArrayList<String>() : delegate.getEagerInitModuleNames();
if (delegate != null) {
synchronized (delegate) {
mEagerInitModuleNames.addAll(delegate.getEagerInitModuleNames());
}
}
mJavaModuleProvider =
new TurboModuleProvider() {
@@ -75,7 +78,10 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
return null;
}
return delegate.getModule(moduleName);
/** TODO(T65532092): Should TurboModuleManagerDelegate be thread-safe? */
synchronized (delegate) {
return delegate.getModule(moduleName);
}
}
};
@@ -87,7 +93,13 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
return null;
}
CxxModuleWrapper nativeModule = delegate.getLegacyCxxModule(moduleName);
CxxModuleWrapper nativeModule;
/** TODO(T65532092): Should TurboModuleManagerDelegate be thread-safe? */
synchronized (delegate) {
nativeModule = delegate.getLegacyCxxModule(moduleName);
}
if (nativeModule != null) {
// TurboModuleManagerDelegate must always return TurboModules
Assertions.assertCondition(