Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NodeAPI] TurboModule jsRespresentation_ cache fails to set property #14128

Open
acoates-ms opened this issue Nov 21, 2024 · 1 comment
Open
Assignees
Labels
Area: Turbo Modules bug New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric
Milestone

Comments

@acoates-ms
Copy link
Contributor

When getting a turbomodule, RN creates a JS object wrapper that it uses to cache the results of __turboModuleProxy.get :
https://github.com/facebook/react-native/blob/48ea6867a96bd16dc7aed9af5a8e9ce12a487c22/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.cpp#L178

Here it creates a jsi::Object, which it stores in the TurboModules jsRepresentation_ property. It then sets the new Object's 'proto' property to the actual turbomodule hostobject. then returns that jsi::Object to JS.

Now when the JS gets a value from the TurboModule it goes to TurboModule::get, which uses TurboModule::create to get the value of the property, then attempts to cache the result by setting the result in the jsRepresentation_ object that it created earlier:
https://github.com/facebook/react-native/blob/48ea6867a96bd16dc7aed9af5a8e9ce12a487c22/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.h#L64

But for some reason, that set ends up in HostObject::set, which throws:
https://github.com/facebook/react-native/blob/48ea6867a96bd16dc7aed9af5a8e9ce12a487c22/packages/react-native/ReactCommon/jsi/jsi/jsi.cpp#L179

I dont understand why it gets into HostObject::set, as the jsRepresentation_ object was just a plain jsi::Object. I wouldn't expect a set on the jsi::Object to end up in HostObject::set.

	Microsoft.ReactNative.dll!facebook::jsi::HostObject::set(facebook::jsi::Runtime & rt={...}, const facebook::jsi::PropNameID & name={...}, const facebook::jsi::Value & __formal={...}) Line 85	C++
	Microsoft.ReactNative.dll!Microsoft::NodeApiJsi::`anonymous-namespace'::NodeApiJsiRuntime::hostObjectSetTrap::__l2::<lambda_1>::operator()() Line 2469	C++
	Microsoft.ReactNative.dll!Microsoft::NodeApiJsi::`anonymous namespace'::NodeApiJsiRuntime::runInMethodContext<`Microsoft::NodeApiJsi::`anonymous namespace'::NodeApiJsiRuntime::hostObjectSetTrap'::`2'::<lambda_1>>(const char * methodName=0x00007fff953f9bb0, Microsoft::NodeApiJsi::`anonymous-namespace'::NodeApiJsiRuntime::hostObjectSetTrap::__l2::<lambda_1> lambda={...}) Line 2006	C++
	Microsoft.ReactNative.dll!Microsoft::NodeApiJsi::`anonymous namespace'::NodeApiJsiRuntime::hostObjectSetTrap(std::span<napi_value__ *,-1> args={...}) Line 2468	C++
	Microsoft.ReactNative.dll!Microsoft::NodeApiJsi::`anonymous-namespace'::NodeApiJsiRuntime::setProxyTrap::__l2::<lambda_1>::()::__l2::<lambda_1>::operator()() Line 2417	C++
	Microsoft.ReactNative.dll!Microsoft::NodeApiJsi::`anonymous namespace'::NodeApiJsiRuntime::handleCallbackExceptions<``Microsoft::NodeApiJsi::`anonymous namespace'::NodeApiJsiRuntime::setProxyTrap<&Microsoft::NodeApiJsi::`anonymous namespace'::NodeApiJsiRuntime::hostObjectSetTrap,4>'::`2'::<lambda_1>::operator()'::`2'::<lambda_1>>(Microsoft::NodeApiJsi::`anonymous-namespace'::NodeApiJsiRuntime::setProxyTrap::__l2::<lambda_1>::()::__l2::<lambda_1> lambda={...}) Line 2021	C++
	Microsoft.ReactNative.dll!Microsoft::NodeApiJsi::`anonymous-namespace'::NodeApiJsiRuntime::setProxyTrap::__l2::<lambda_1>::operator()(napi_env__ * env=0x00000236b9b2c7c0, napi_callback_info__ * info=0x00000093bcffc320) Line 2416	C++
	Microsoft.ReactNative.dll!`Microsoft::NodeApiJsi::`anonymous namespace'::NodeApiJsiRuntime::setProxyTrap<&Microsoft::NodeApiJsi::`anonymous namespace'::NodeApiJsiRuntime::hostObjectSetTrap,4>'::`2'::<lambda_1>::<lambda_invoker_cdecl>(napi_env__ * env=0x00000236b9b2c7c0, napi_callback_info__ * info=0x00000093bcffc320) Line 2418	C++
	[Inline Frame] hermes.dll!hermes::napi::NapiHostFunctionContext::func::__l2::<lambda_a5336cdf707cb3b82a410896b6e2d4bf>::operator()(hermes::napi::NapiEnvironment *) Line 2061	C++
	[Inline Frame] hermes.dll!hermes::napi::NapiEnvironment::callIntoModule(hermes::napi::NapiHostFunctionContext::func::__l2::<lambda_a5336cdf707cb3b82a410896b6e2d4bf> &&) Line 4977	C++
	hermes.dll!hermes::napi::NapiHostFunctionContext::func(void * context, hermes::vm::Runtime & runtime, hermes::vm::NativeArgs hvArgs) Line 2060	C++
	[Inline Frame] hermes.dll!hermes::vm::NativeFunction::_nativeCall(hermes::vm::NativeFunction * self=0x00000236d642d118, hermes::vm::Runtime &) Line 507	C++
	hermes.dll!hermes::vm::NativeFunction::_callImpl(hermes::vm::Handle<hermes::vm::Callable> selfHandle, hermes::vm::Runtime & runtime={...}) Line 938	C++
	[Inline Frame] hermes.dll!hermes::vm::Callable::call(hermes::vm::Handle<hermes::vm::Callable>) Line 253	C++
	hermes.dll!hermes::vm::Callable::executeCall4(hermes::vm::Handle<hermes::vm::Callable> selfHandle={...}, hermes::vm::Runtime & runtime={...}, hermes::vm::Handle<hermes::vm::HermesValue> thisArgHandle, hermes::vm::HermesValue param1={...}, hermes::vm::HermesValue param2={...}, hermes::vm::HermesValue param3={...}, hermes::vm::HermesValue param4={...}, bool construct=false) Line 318	C++
	hermes.dll!hermes::vm::`anonymous namespace'::setWithTrap(hermes::vm::Runtime & runtime={...}, hermes::vm::Handle<hermes::vm::HermesValue> nameValHandle={...}, hermes::vm::Handle<hermes::vm::HermesValue> valueHandle={...}, hermes::vm::Handle<hermes::vm::Callable> trap={...}, hermes::vm::Handle<hermes::vm::JSObject> handler={...}, hermes::vm::Handle<hermes::vm::JSObject> target={...}, hermes::vm::Handle<hermes::vm::HermesValue> receiver={...}) Line 1022	C++
	hermes.dll!hermes::vm::JSProxy::setComputed(hermes::vm::Handle<hermes::vm::JSObject> selfHandle, hermes::vm::Runtime & runtime={...}, hermes::vm::Handle<hermes::vm::HermesValue> nameValHandle={...}, hermes::vm::Handle<hermes::vm::HermesValue> valueHandle={...}, hermes::vm::Handle<hermes::vm::HermesValue> receiver={...}) Line 1139	C++
	hermes.dll!hermes::vm::JSObject::putComputedWithReceiver_RJS(hermes::vm::Handle<hermes::vm::JSObject> selfHandle={...}, hermes::vm::Runtime & runtime={...}, hermes::vm::Handle<hermes::vm::HermesValue> nameValHandle, hermes::vm::Handle<hermes::vm::HermesValue> valueHandle={...}, hermes::vm::Handle<hermes::vm::HermesValue> receiver={...}, hermes::vm::PropOpFlags opFlags={...}) Line 1663	C++
	[Inline Frame] hermes.dll!hermes::vm::JSObject::putComputed_RJS(hermes::vm::Handle<hermes::vm::JSObject>) Line 1960	C++
	[Inline Frame] hermes.dll!hermes::napi::NapiEnvironment::setComputedProperty(napi_value__ *) Line 5128	C++
	hermes.dll!hermes::napi::NapiEnvironment::setProperty(napi_value__ * object=0x00000236b9fc9c20, napi_value__ * key=0x00000236c8e40b48, napi_value__ * value=0x00000236b9fc9c18) Line 4524	C++
	Microsoft.ReactNative.dll!Microsoft::NodeApiJsi::`anonymous namespace'::NodeApiJsiRuntime::setProperty(napi_value__ * object=0x00000236b9fc9c20, napi_value__ * propertyId=0x00000236c8e40b48, napi_value__ * value=0x00000236b9fc9c18) Line 2256	C++
	Microsoft.ReactNative.dll!Microsoft::NodeApiJsi::`anonymous namespace'::NodeApiJsiRuntime::setPropertyValue(const facebook::jsi::Object & obj={...}, const facebook::jsi::PropNameID & name={...}, const facebook::jsi::Value & value={...}) Line 1382	C++
	Microsoft.ReactNative.dll!facebook::jsi::Object::setPropertyValue(facebook::jsi::Runtime & runtime={...}, const facebook::jsi::PropNameID & name={...}, const facebook::jsi::Value & value={...}) Line 932	C++
	Microsoft.ReactNative.dll!facebook::jsi::Object::setProperty<facebook::jsi::Value &>(facebook::jsi::Runtime & runtime={...}, const facebook::jsi::PropNameID & name={...}, facebook::jsi::Value & value={...}) Line 132	C++
	Microsoft.ReactNative.dll!facebook::react::TurboModule::get(facebook::jsi::Runtime & runtime={...}, const facebook::jsi::PropNameID & propName={...}) Line 65	C++
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Nov 21, 2024
@chrisglein chrisglein added bug Area: Turbo Modules and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Nov 21, 2024
@chrisglein chrisglein added this to the Next milestone Nov 21, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric label Nov 21, 2024
@chrisglein
Copy link
Member

Workaround in place that bypasses the cache. So working, but slower.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Turbo Modules bug New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric
Projects
None yet
Development

No branches or pull requests

3 participants