facebook / hermes

A JavaScript engine optimized for running React Native.
https://hermesengine.dev/
MIT License
9.41k stars 596 forks source link

Closures caputure external variable errors #1360

Closed cs-robert closed 1 month ago

cs-robert commented 1 month ago

Bug Description

I packaged the js code using webpack and executed it with the hermes engine, but the results didn't work as expected

Hermes git revision (if applicable): 0.6.0

OS:Android

my_engineGlobal should be v1 but it's value is not

code example:

图片

demo https://github.com/cs-robert/hermesTest

run demo ,click test ,adb logcat | grep JsNativeLog The originGlobal value is really weird

tmikov commented 1 month ago

I am sorry, can you please provide a minimal reproduction of the problem using the Hermes CLI?

cs-robert commented 1 month ago

I am sorry, can you please provide a minimal reproduction of the problem using the Hermes CLI?

The same code executed directly by the hermes command on a mac is fine, but the results of a jsi call on android are not as expected (modify /HermesTest/src/main/assets/test.js JsNativeLog(msg);=>print(msg); test.js can be executed directly with the hermes command)

图片
cs-robert commented 1 month ago

I am sorry, can you please provide a minimal reproduction of the problem using the Hermes CLI?

The logic of executing js is very simple, just inject a JsNativeLog method to print the log, and then execute test.js all code is in https://github.com/cs-robert/hermesTest ` Java_com_example_hermestest_MainActivity_evaluateJavaScript(JNIEnv env, jclass clazz, jstring script) { try { std::shared_ptr runtime = facebook::hermes::makeHermesRuntime(); runtime->global().setProperty(runtime, "JsNativeLog", facebook::jsi::Function::createFromHostFunction( runtime, facebook::jsi::PropNameID::forAscii(runtime, "JsNativeLog"), 0, [](facebook::jsi::Runtime &rt, const facebook::jsi::Value &thisVal, const facebook::jsi::Value arguments, size_t argumentCount) { auto str = arguments[0].getString(rt).utf8(rt); __android_log_print(ANDROID_LOG_INFO, "JsNativeLog", "JSLog %s", str.c_str()); return facebook::jsi::Value(); })); const char scriptChar = env->GetStringUTFChars(script, nullptr); std::string source = std::string(scriptChar); std::shared_ptr hbcPtr = std::make_unique(source); runtime->evaluateJavaScript(hbcPtr, "test.js"); } catch (const facebook::jsi::JSError &error) { android_log_print(ANDROID_LOG_ERROR, "JsNativeLog", "%s %s", error.getMessage().c_str(), error.getStack().c_str()); } catch (const facebook::jsi::JSIException &err) { android_log_print(ANDROID_LOG_ERROR, "JsNativeLog", "%s", err.what());

} catch (const std::exception &ex) {
    __android_log_print(ANDROID_LOG_ERROR, "JsNativeLog", "%s", ex.what());

} catch (...) {

}

} `

cs-robert commented 1 month ago

I am sorry, can you please provide a minimal reproduction of the problem using the Hermes CLI?

Strangely enough, when I delete some of the WebPack-generated code in the js file, it works fine.What I'm removing is a closure that won't execute and doesn't affect the logic。 my_engineGlobal, my_engineGlobal2, my_engineGlobal3, my_engineGlobal4... These variables are simply defined and printed in a method without any logic to modify them

cs-robert commented 1 month ago

image

cs-robert commented 1 month ago

hbc doesn't seem to have this problem, not sure if it's a coincidence or really OK

tmikov commented 1 month ago

@cs-robert in your first post you said Hermes version 0.6.0. That version is almost 4 years old. Can you reproduce this with the latest Hermes version?

cs-robert commented 1 month ago

@cs-robert in your first post you said Hermes version 0.6.0. That version is almost 4 years old. Can you reproduce this with the latest Hermes version?

@tmikov 0.7.2 work fine,Why is hbc OK? And want to understand the specific logic of what caused it? How was it repaired?

tmikov commented 1 month ago

@cs-robert it is hard to say, many things have changed in 4 years. Still, resolution of variables is very important, so I am pretty sure that even old versions didn't have major bugs related to it. You might be seeing a problem related to lazy compilation in an unexpected context: when running from source, Hermes lazily compiles every function when it is called the first time, which is a complicated process.