Skip to content

Commit

Permalink
Fix incomplete UI runtime initialization (#5505)
Browse files Browse the repository at this point in the history
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

Fixes #5421, fixes #5331.

This PR removes `installValueUnpacker` method which was called on the JS
thread during the creation of `NativeReanimatedModule` but operated
directly on the UI runtime. Because of this, UI runtime was already used
from JS thread so any use from the UI runtime (e.g. Reanimated event
handlers) would cause a crash due to a failed check in
`ReanimatedReentrancyCheck::before`. Note that the check also occurred
in release mode due to improperly set `NDEBUG` flag.

## Test plan

<!-- Provide a minimal but complete code snippet that can be used to
test out this change along with instructions how to run it and a
description of the expected behavior. -->
  • Loading branch information
tomekzaw committed Dec 21, 2023
1 parent 5b4cd51 commit 5d1c254
Show file tree
Hide file tree
Showing 16 changed files with 63 additions and 85 deletions.
21 changes: 10 additions & 11 deletions Common/cpp/NativeModules/NativeReanimatedModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ NativeReanimatedModule::NativeReanimatedModule(
const std::shared_ptr<CallInvoker> &jsInvoker,
const std::shared_ptr<MessageQueueThread> &jsQueue,
const std::shared_ptr<UIScheduler> &uiScheduler,
const PlatformDepMethodsHolder &platformDepMethodsHolder)
const PlatformDepMethodsHolder &platformDepMethodsHolder,
const std::string &valueUnpackerCode)
: NativeReanimatedModuleSpec(jsInvoker),
jsQueue_(jsQueue),
jsScheduler_(std::make_shared<JSScheduler>(rnRuntime, jsInvoker)),
Expand All @@ -58,7 +59,9 @@ NativeReanimatedModule::NativeReanimatedModule(
rnRuntime,
jsQueue,
jsScheduler_,
"Reanimated UI runtime")),
"Reanimated UI runtime",
valueUnpackerCode)),
valueUnpackerCode_(valueUnpackerCode),
eventHandlerRegistry_(std::make_unique<EventHandlerRegistry>()),
requestRender_(platformDepMethodsHolder.requestRender),
onRenderCallback_([this](const double timestampMs) {
Expand Down Expand Up @@ -132,13 +135,6 @@ NativeReanimatedModule::NativeReanimatedModule(
platformDepMethodsHolder.maybeFlushUIUpdatesQueueFunction);
}

void NativeReanimatedModule::installValueUnpacker(
jsi::Runtime &rt,
const jsi::Value &valueUnpackerCode) {
valueUnpackerCode_ = valueUnpackerCode.asString(rt).utf8(rt);
uiWorkletRuntime_->installValueUnpacker(valueUnpackerCode_);
}

NativeReanimatedModule::~NativeReanimatedModule() {
// event handler registry and frame callbacks store some JSI values from UI
// runtime, so they have to go away before we tear down the runtime
Expand Down Expand Up @@ -170,8 +166,11 @@ jsi::Value NativeReanimatedModule::createWorkletRuntime(
const jsi::Value &name,
const jsi::Value &initializer) {
auto workletRuntime = std::make_shared<WorkletRuntime>(
rt, jsQueue_, jsScheduler_, name.asString(rt).utf8(rt));
workletRuntime->installValueUnpacker(valueUnpackerCode_);
rt,
jsQueue_,
jsScheduler_,
name.asString(rt).utf8(rt),
valueUnpackerCode_);
auto initializerShareable = extractShareableOrThrow<ShareableWorklet>(
rt, initializer, "[Reanimated] Initializer must be a worklet.");
workletRuntime->runGuarded(initializerShareable);
Expand Down
7 changes: 2 additions & 5 deletions Common/cpp/NativeModules/NativeReanimatedModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,11 @@ class NativeReanimatedModule : public NativeReanimatedModuleSpec {
const std::shared_ptr<CallInvoker> &jsInvoker,
const std::shared_ptr<MessageQueueThread> &jsQueue,
const std::shared_ptr<UIScheduler> &uiScheduler,
const PlatformDepMethodsHolder &platformDepMethodsHolder);
const PlatformDepMethodsHolder &platformDepMethodsHolder,
const std::string &valueUnpackerCode);

~NativeReanimatedModule();

void installValueUnpacker(
jsi::Runtime &rt,
const jsi::Value &valueUnpackerCode) override;

jsi::Value makeShareableClone(
jsi::Runtime &rt,
const jsi::Value &value,
Expand Down
13 changes: 0 additions & 13 deletions Common/cpp/NativeModules/NativeReanimatedModuleSpec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,6 @@

namespace reanimated {

static jsi::Value SPEC_PREFIX(installValueUnpacker)(
jsi::Runtime &rt,
TurboModule &turboModule,
const jsi::Value *args,
size_t) {
static_cast<NativeReanimatedModuleSpec *>(&turboModule)
->installValueUnpacker(rt, std::move(args[0]));
return jsi::Value::undefined();
}

// SharedValue

static jsi::Value SPEC_PREFIX(makeShareableClone)(
Expand Down Expand Up @@ -198,9 +188,6 @@ static jsi::Value SPEC_PREFIX(setShouldAnimateExiting)(
NativeReanimatedModuleSpec::NativeReanimatedModuleSpec(
std::shared_ptr<CallInvoker> jsInvoker)
: TurboModule("NativeReanimated", jsInvoker) {
methodMap_["installValueUnpacker"] =
MethodMetadata{1, SPEC_PREFIX(installValueUnpacker)};

methodMap_["makeShareableClone"] =
MethodMetadata{2, SPEC_PREFIX(makeShareableClone)};

Expand Down
4 changes: 0 additions & 4 deletions Common/cpp/NativeModules/NativeReanimatedModuleSpec.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ class JSI_EXPORT NativeReanimatedModuleSpec : public TurboModule {
explicit NativeReanimatedModuleSpec(std::shared_ptr<CallInvoker> jsInvoker);

public:
virtual void installValueUnpacker(
jsi::Runtime &rt,
const jsi::Value &valueUnpackerCode) = 0;

// SharedValue
virtual jsi::Value makeShareableClone(
jsi::Runtime &rt,
Expand Down
14 changes: 6 additions & 8 deletions Common/cpp/ReanimatedRuntime/WorkletRuntime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,19 @@ WorkletRuntime::WorkletRuntime(
jsi::Runtime &rnRuntime,
const std::shared_ptr<MessageQueueThread> &jsQueue,
const std::shared_ptr<JSScheduler> &jsScheduler,
const std::string &name)
const std::string &name,
const std::string &valueUnpackerCode)
: runtime_(ReanimatedRuntime::make(rnRuntime, jsQueue, name)), name_(name) {
jsi::Runtime &rt = *runtime_;
WorkletRuntimeCollector::install(rt);
WorkletRuntimeDecorator::decorate(rt, name, jsScheduler);
}

void WorkletRuntime::installValueUnpacker(
const std::string &valueUnpackerCode) {
jsi::Runtime &rt = *runtime_;
auto codeBuffer = std::make_shared<const jsi::StringBuffer>(
"(" + valueUnpackerCode + "\n)");
auto valueUnpacker = rt.evaluateJavaScript(codeBuffer, "installValueUnpacker")
.asObject(rt)
.asFunction(rt);
auto valueUnpacker =
rt.evaluateJavaScript(codeBuffer, "WorkletRuntime::WorkletRuntime")
.asObject(rt)
.asFunction(rt);
rt.global().setProperty(rt, "__valueUnpacker", valueUnpacker);
}

Expand Down
5 changes: 2 additions & 3 deletions Common/cpp/ReanimatedRuntime/WorkletRuntime.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ class WorkletRuntime : public jsi::HostObject,
jsi::Runtime &rnRuntime,
const std::shared_ptr<MessageQueueThread> &jsQueue,
const std::shared_ptr<JSScheduler> &jsScheduler,
const std::string &name);

void installValueUnpacker(const std::string &valueUnpackerCode);
const std::string &name,
const std::string &valueUnpackerCode);

jsi::Runtime &getJSIRuntime() const {
return *runtime_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class NativeProxy extends NativeProxyCommon {
@SuppressWarnings("unused")
private final HybridData mHybridData;

public NativeProxy(ReactApplicationContext context) {
public NativeProxy(ReactApplicationContext context, String valueUnpackerCode) {
super(context);
ReactFeatureFlagsWrapper.enableMountHooks();
CallInvokerHolderImpl holder =
Expand All @@ -39,7 +39,8 @@ public NativeProxy(ReactApplicationContext context) {
mAndroidUIScheduler,
LayoutAnimations,
messageQueueThread,
fabricUIManager);
fabricUIManager,
valueUnpackerCode);
prepareLayoutAnimations(LayoutAnimations);
installJSIBindings();
if (BuildConfig.DEBUG) {
Expand All @@ -53,7 +54,8 @@ private native HybridData initHybrid(
AndroidUIScheduler androidUIScheduler,
LayoutAnimations LayoutAnimations,
MessageQueueThread messageQueueThread,
FabricUIManager fabricUIManager);
FabricUIManager fabricUIManager,
String valueUnpackerCode);

public native boolean isAnyHandlerWaitingForEvent(String eventName, int emitterReactTag);

Expand Down
24 changes: 11 additions & 13 deletions android/src/main/cpp/NativeProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,21 @@ NativeProxy::NativeProxy(
const std::shared_ptr<facebook::react::CallInvoker> &jsCallInvoker,
const std::shared_ptr<UIScheduler> &uiScheduler,
jni::global_ref<LayoutAnimations::javaobject> layoutAnimations,
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread,
#ifdef RCT_NEW_ARCH_ENABLED
,
jni::alias_ref<facebook::react::JFabricUIManager::javaobject>
fabricUIManager
fabricUIManager,
#endif
)
const std::string &valueUnpackerCode)
: javaPart_(jni::make_global(jThis)),
rnRuntime_(rnRuntime),
nativeReanimatedModule_(std::make_shared<NativeReanimatedModule>(
*rnRuntime,
jsCallInvoker,
std::make_shared<JMessageQueueThread>(messageQueueThread),
uiScheduler,
getPlatformDependentMethods())),
getPlatformDependentMethods(),
valueUnpackerCode)),
layoutAnimations_(std::move(layoutAnimations)) {
#ifdef RCT_NEW_ARCH_ENABLED
const auto &uiManager =
Expand Down Expand Up @@ -85,13 +85,12 @@ jni::local_ref<NativeProxy::jhybriddata> NativeProxy::initHybrid(
jsCallInvokerHolder,
jni::alias_ref<AndroidUIScheduler::javaobject> androidUiScheduler,
jni::alias_ref<LayoutAnimations::javaobject> layoutAnimations,
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread,
#ifdef RCT_NEW_ARCH_ENABLED
,
jni::alias_ref<facebook::react::JFabricUIManager::javaobject>
fabricUIManager
fabricUIManager,
#endif
) {
const std::string &valueUnpackerCode) {
auto jsCallInvoker = jsCallInvokerHolder->cthis()->getCallInvoker();
auto uiScheduler = androidUiScheduler->cthis()->getUIScheduler();
return makeCxxInstance(
Expand All @@ -100,12 +99,11 @@ jni::local_ref<NativeProxy::jhybriddata> NativeProxy::initHybrid(
jsCallInvoker,
uiScheduler,
make_global(layoutAnimations),
messageQueueThread
messageQueueThread,
#ifdef RCT_NEW_ARCH_ENABLED
,
fabricUIManager
fabricUIManager,
#endif
/**/);
valueUnpackerCode);
}

#ifndef NDEBUG
Expand Down
14 changes: 6 additions & 8 deletions android/src/main/cpp/NativeProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,12 @@ class NativeProxy : public jni::HybridClass<NativeProxy> {
jsCallInvokerHolder,
jni::alias_ref<AndroidUIScheduler::javaobject> androidUiScheduler,
jni::alias_ref<LayoutAnimations::javaobject> layoutAnimations,
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread,
#ifdef RCT_NEW_ARCH_ENABLED
,
jni::alias_ref<facebook::react::JFabricUIManager::javaobject>
fabricUIManager
fabricUIManager,
#endif
/**/);
const std::string &valueUnpackerCode);
static void registerNatives();

~NativeProxy();
Expand Down Expand Up @@ -266,13 +265,12 @@ class NativeProxy : public jni::HybridClass<NativeProxy> {
const std::shared_ptr<facebook::react::CallInvoker> &jsCallInvoker,
const std::shared_ptr<UIScheduler> &uiScheduler,
jni::global_ref<LayoutAnimations::javaobject> layoutAnimations,
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread,
#ifdef RCT_NEW_ARCH_ENABLED
,
jni::alias_ref<facebook::react::JFabricUIManager::javaobject>
fabricUIManager
fabricUIManager,
#endif
/**/);
const std::string &valueUnpackerCode);
};

} // namespace reanimated
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,10 @@ public void onCatalystInstanceDestroy() {
}
}

public void initWithContext(ReactApplicationContext reactApplicationContext) {
public void initWithContext(
ReactApplicationContext reactApplicationContext, String valueUnpackerCode) {
mReactApplicationContext = reactApplicationContext;
mNativeProxy = new NativeProxy(reactApplicationContext);
mNativeProxy = new NativeProxy(reactApplicationContext, valueUnpackerCode);
mAnimationManager.setAndroidUIScheduler(getNativeProxy().getAndroidUIScheduler());
compatibility = new ReaCompatibility(reactApplicationContext);
compatibility.registerFabricEventListener(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@ public NodesManager getNodesManager() {
}

@ReactMethod(isBlockingSynchronousMethod = true)
public void installTurboModule() {
public void installTurboModule(String valueUnpackerCode) {
// When debugging in chrome the JS context is not available.
// https://github.com/facebook/react-native/blob/v0.67.0-rc.6/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.java#L25
Utils.isChromeDebugger = getReactApplicationContext().getJavaScriptContextHolder().get() == 0;

if (!Utils.isChromeDebugger) {
this.getNodesManager().initWithContext(getReactApplicationContext());
this.getNodesManager().initWithContext(getReactApplicationContext(), valueUnpackerCode);
} else {
Log.w(
"[REANIMATED]",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class NativeProxy extends NativeProxyCommon {
@SuppressWarnings("unused")
private final HybridData mHybridData;

public NativeProxy(ReactApplicationContext context) {
public NativeProxy(ReactApplicationContext context, String valueUnpackerCode) {
super(context);
CallInvokerHolderImpl holder =
(CallInvokerHolderImpl) context.getCatalystInstance().getJSCallInvokerHolder();
Expand All @@ -31,7 +31,8 @@ public NativeProxy(ReactApplicationContext context) {
holder,
mAndroidUIScheduler,
LayoutAnimations,
messageQueueThread);
messageQueueThread,
valueUnpackerCode);
prepareLayoutAnimations(LayoutAnimations);
installJSIBindings();
if (BuildConfig.DEBUG) {
Expand All @@ -44,7 +45,8 @@ private native HybridData initHybrid(
CallInvokerHolderImpl jsCallInvokerHolder,
AndroidUIScheduler androidUIScheduler,
LayoutAnimations LayoutAnimations,
MessageQueueThread messageQueueThread);
MessageQueueThread messageQueueThread,
String valueUnpackerCode);

public native boolean isAnyHandlerWaitingForEvent(String eventName, int emitterReactTag);

Expand Down
5 changes: 3 additions & 2 deletions apple/REAModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -257,14 +257,15 @@ - (void)sendEventWithName:(NSString *)eventName body:(id)body
}
}

RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD(installTurboModule)
RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD(installTurboModule : (nonnull NSString *)valueUnpackerCode)
{
facebook::jsi::Runtime *jsiRuntime = [self.bridge respondsToSelector:@selector(runtime)]
? reinterpret_cast<facebook::jsi::Runtime *>(self.bridge.runtime)
: nullptr;

if (jsiRuntime) {
auto nativeReanimatedModule = reanimated::createReanimatedModule(self.bridge, self.bridge.jsCallInvoker);
auto nativeReanimatedModule = reanimated::createReanimatedModule(
self.bridge, self.bridge.jsCallInvoker, std::string([valueUnpackerCode UTF8String]));

jsi::Runtime &rnRuntime = *jsiRuntime;
WorkletRuntimeCollector::install(rnRuntime);
Expand Down
3 changes: 2 additions & 1 deletion apple/native/NativeProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ namespace reanimated {

std::shared_ptr<reanimated::NativeReanimatedModule> createReanimatedModule(
RCTBridge *bridge,
const std::shared_ptr<facebook::react::CallInvoker> &jsInvoker);
const std::shared_ptr<facebook::react::CallInvoker> &jsInvoker,
const std::string &valueUnpackerCode);

}

Expand Down
7 changes: 4 additions & 3 deletions apple/native/NativeProxy.mm
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ static CFTimeInterval calculateTimestampWithSlowAnimations(CFTimeInterval curren

std::shared_ptr<NativeReanimatedModule> createReanimatedModule(
RCTBridge *bridge,
const std::shared_ptr<CallInvoker> &jsInvoker)
const std::shared_ptr<CallInvoker> &jsInvoker,
const std::string &valueUnpackerCode)
{
REAModule *reaModule = [bridge moduleForClass:[REAModule class]];

Expand Down Expand Up @@ -287,8 +288,8 @@ static CFTimeInterval calculateTimestampWithSlowAnimations(CFTimeInterval curren
maybeFlushUIUpdatesQueueFunction,
};

auto nativeReanimatedModule =
std::make_shared<NativeReanimatedModule>(rnRuntime, jsInvoker, jsQueue, uiScheduler, platformDepMethodsHolder);
auto nativeReanimatedModule = std::make_shared<NativeReanimatedModule>(
rnRuntime, jsInvoker, jsQueue, uiScheduler, platformDepMethodsHolder, valueUnpackerCode);

[reaModule.nodesManager registerEventHandler:^(id<RCTEvent> event) {
// handles RCTEvents from RNGestureHandler
Expand Down

0 comments on commit 5d1c254

Please sign in to comment.