Skip to content

Commit

Permalink
Implement "tickleJS" for Hermes (#39289)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #39289

"tickleJS" is a function in RN Hermes Debugger, we need to implement it for Bridgeless for feature correctness and keep parity with Bridge.

Changelog:
[Internal] -  Implement "tickleJS" for Hermes in Bridgeless mode

### Summary of discussion:

**Goal of this diff**:
Implement tickleJS for Hermes debugger with Bridgeless, here is Bridge's implementation
https://www.internalfb.com/code/fbsource/[7058fb4dae4f]/xplat/ReactNative/react/jsi/HermesExecutorFactory.cpp?lines=45-57

**The key problem to solve:**
As you can see from Bridge's implementation, we need to dispatch the js call of "__tickleJs" to dedicated JS thread queue for thread safety, and it did cause a crash for me locally without dispatching to JS thread queue (see P818439922).

For Bridgeless passing the JS message queue directly works, but then we would naturally want ask if RuntimeExecutor/RuntimeScheduler is a better choice because: 1, It should also work 2, It's the common way to run js code in native with Bridgeless 3, Can avoid potential race condition caused by dispatching to JS message queue directly 4, We're planning to expose RuntimeExecutor/RuntimeScheduler or a similar abstraction anyway. So the argument here is which solution should we take?

**Why we decide to move forward with the JS msq solution?**
- Time constraint: we need to ship this before 0.73
- After discussion there hasn't been any major concern over dispatching to JS msq directly so far
- How to expose RuntimeExecutor/RuntimeScheduler is a bigger topic which need further research, we don't want to hold this diff for it

For detailed discussion see the post https://fb.workplace.com/groups/react.technologies.discussions/permalink/3615044702060575/

Reviewed By: javache

Differential Revision: D48952581

fbshipit-source-id: 3dfb05d35c422f3fd6a420e265e4efe8fa13f057
  • Loading branch information
Lulu Wu authored and facebook-github-bot committed Sep 13, 2023
1 parent 5a926c5 commit fe4dab9
Show file tree
Hide file tree
Showing 12 changed files with 61 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ void JHermesInstance::registerNatives() {
});
}

std::unique_ptr<jsi::Runtime> JHermesInstance::createJSRuntime() noexcept {
std::unique_ptr<jsi::Runtime> JHermesInstance::createJSRuntime(
std::shared_ptr<MessageQueueThread> msgQueueThread) noexcept {
// TODO T105438175 Pass ReactNativeConfig to init Hermes with MobileConfig
return HermesInstance::createJSRuntime();
return HermesInstance::createJSRuntime(nullptr, nullptr, msgQueueThread);
}

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <memory>
#include <string>

#include <cxxreact/MessageQueueThread.h>
#include <fbjni/fbjni.h>
#include <jni.h>
#include <jsi/jsi.h>
Expand All @@ -29,7 +30,8 @@ class JHermesInstance

static void registerNatives();

std::unique_ptr<jsi::Runtime> createJSRuntime() noexcept;
std::unique_ptr<jsi::Runtime> createJSRuntime(
std::shared_ptr<MessageQueueThread> msgQueueThread) noexcept;

~JHermesInstance() {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ JReactInstance::JReactInstance(
jBindingsInstaller_ = jni::make_global(jBindingsInstaller);

instance_ = std::make_unique<ReactInstance>(
jsEngineInstance->cthis()->createJSRuntime(),
jsEngineInstance->cthis()->createJSRuntime(sharedJSMessageQueueThread),
sharedJSMessageQueueThread,
timerManager,
std::move(jsErrorHandlingFunc));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

#include <cxxreact/MessageQueueThread.h>
#include <fbjni/fbjni.h>
#include <jsc/JSCRuntime.h>
#include <jsi/jsi.h>
Expand All @@ -29,7 +30,8 @@ class JSCInstance : public jni::HybridClass<JSCInstance, JJSEngineInstance> {
});
}

std::unique_ptr<jsi::Runtime> createJSRuntime() noexcept {
std::unique_ptr<jsi::Runtime> createJSRuntime(
std::shared_ptr<MessageQueueThread> msgQueueThread) noexcept {
return jsc::makeJSCRuntime();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#pragma once

#include <ReactCommon/RuntimeExecutor.h>
#include <cxxreact/MessageQueueThread.h>
#include <jsi/jsi.h>

namespace facebook::react {
Expand All @@ -17,7 +18,8 @@ namespace facebook::react {
*/
class JSEngineInstance {
public:
virtual std::unique_ptr<jsi::Runtime> createJSRuntime() noexcept = 0;
virtual std::unique_ptr<jsi::Runtime> createJSRuntime(
std::shared_ptr<MessageQueueThread> msgQueueThread) noexcept = 0;

virtual ~JSEngineInstance() = default;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,43 @@ namespace facebook::react {
// lead to a runtime crash.
class HermesInstanceRuntimeAdapter : public inspector_modern::RuntimeAdapter {
public:
HermesInstanceRuntimeAdapter(std::shared_ptr<HermesRuntime> hermesRuntime)
: hermesRuntime_(hermesRuntime) {}
HermesInstanceRuntimeAdapter(
std::shared_ptr<HermesRuntime> hermesRuntime,
std::shared_ptr<MessageQueueThread> msgQueueThread)
: hermesRuntime_(std::move(hermesRuntime)),
messageQueueThread_(std::move(msgQueueThread)) {}
virtual ~HermesInstanceRuntimeAdapter() = default;

HermesRuntime& getRuntime() override {
return *hermesRuntime_;
}

void tickleJs() override {
std::weak_ptr<HermesRuntime> weakRuntime(hermesRuntime_);
messageQueueThread_->runOnQueue([weakRuntime]() {
auto runtime = weakRuntime.lock();
if (!runtime) {
return;
}
jsi::Function func =
runtime->global().getPropertyAsFunction(*runtime, "__tickleJs");
func.call(*runtime);
});
}

private:
std::shared_ptr<HermesRuntime> hermesRuntime_;
std::shared_ptr<MessageQueueThread> messageQueueThread_;
};

class DecoratedRuntime : public jsi::RuntimeDecorator<jsi::Runtime> {
public:
DecoratedRuntime(std::unique_ptr<HermesRuntime> runtime)
DecoratedRuntime(
std::unique_ptr<HermesRuntime> runtime,
std::shared_ptr<MessageQueueThread> msgQueueThread)
: RuntimeDecorator<jsi::Runtime>(*runtime), runtime_(std::move(runtime)) {
auto adapter = std::make_unique<HermesInstanceRuntimeAdapter>(runtime_);
auto adapter = std::make_unique<HermesInstanceRuntimeAdapter>(
runtime_, msgQueueThread);

debugToken_ = inspector_modern::chrome::enableDebugging(
std::move(adapter), "Hermes Bridgeless React Native");
Expand All @@ -70,13 +90,11 @@ class DecoratedRuntime : public jsi::RuntimeDecorator<jsi::Runtime> {

#endif

std::unique_ptr<jsi::Runtime> HermesInstance::createJSRuntime() noexcept {
return createJSRuntime(nullptr, nullptr);
}

std::unique_ptr<jsi::Runtime> HermesInstance::createJSRuntime(
std::shared_ptr<const ReactNativeConfig> reactNativeConfig,
std::shared_ptr<::hermes::vm::CrashManager> cm) noexcept {
std::shared_ptr<::hermes::vm::CrashManager> cm,
std::shared_ptr<MessageQueueThread> msgQueueThread) noexcept {
assert(msgQueueThread != nullptr);
int64_t vmExperimentFlags = reactNativeConfig
? reactNativeConfig->getInt64("ios_hermes:vm_experiment_flags")
: 0;
Expand Down Expand Up @@ -113,7 +131,8 @@ std::unique_ptr<jsi::Runtime> HermesInstance::createJSRuntime(

#ifdef HERMES_ENABLE_DEBUGGER
std::unique_ptr<DecoratedRuntime> decoratedRuntime =
std::make_unique<DecoratedRuntime>(std::move(hermesRuntime));
std::make_unique<DecoratedRuntime>(
std::move(hermesRuntime), msgQueueThread);
return decoratedRuntime;
#endif

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#pragma once

#include <cxxreact/MessageQueueThread.h>
#include <hermes/hermes.h>
#include <jsi/jsi.h>
#include <react/config/ReactNativeConfig.h>
Expand All @@ -15,12 +16,10 @@ namespace facebook::react {

class HermesInstance {
public:
// This is only needed for Android. Consider removing.
static std::unique_ptr<jsi::Runtime> createJSRuntime() noexcept;

static std::unique_ptr<jsi::Runtime> createJSRuntime(
std::shared_ptr<const ReactNativeConfig> reactNativeConfig,
std::shared_ptr<::hermes::vm::CrashManager> cm) noexcept;
std::shared_ptr<::hermes::vm::CrashManager> cm,
std::shared_ptr<MessageQueueThread> msgQueueThread) noexcept;
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#import <UIKit/UIKit.h>

#import <cxxreact/MessageQueueThread.h>
#import <hermes/Public/CrashManager.h>
#import <jsi/jsi.h>
#import <react/runtime/JSEngineInstance.h>
Expand All @@ -25,7 +26,8 @@ class RCTHermesInstance : public JSEngineInstance {
std::shared_ptr<const ReactNativeConfig> reactNativeConfig,
CrashManagerProvider crashManagerProvider);

std::unique_ptr<jsi::Runtime> createJSRuntime() noexcept override;
std::unique_ptr<jsi::Runtime> createJSRuntime(
std::shared_ptr<MessageQueueThread> msgQueueThread) noexcept override;

~RCTHermesInstance(){};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@
{
}

std::unique_ptr<jsi::Runtime> RCTHermesInstance::createJSRuntime() noexcept
std::unique_ptr<jsi::Runtime> RCTHermesInstance::createJSRuntime(
std::shared_ptr<MessageQueueThread> msgQueueThread) noexcept
{
return _hermesInstance->createJSRuntime(
_reactNativeConfig, _crashManagerProvider ? _crashManagerProvider() : nullptr);
_reactNativeConfig, _crashManagerProvider ? _crashManagerProvider() : nullptr, msgQueueThread);
}

} // namespace react
} // namespace facebook
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,10 @@ - (void)_start

// Create the React Instance
_reactInstance = std::make_unique<ReactInstance>(
_jsEngineInstance->createJSRuntime(), _jsThreadManager.jsMessageThread, timerManager, jsErrorHandlingFunc);
_jsEngineInstance->createJSRuntime(_jsThreadManager.jsMessageThread),
_jsThreadManager.jsMessageThread,
timerManager,
jsErrorHandlingFunc);
_valid = true;

RuntimeExecutor bufferedRuntimeExecutor = _reactInstance->getBufferedRuntimeExecutor();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

#import <cxxreact/MessageQueueThread.h>
#import <jsi/jsi.h>
#import <react/runtime/JSEngineInstance.h>

Expand All @@ -15,7 +16,8 @@ class RCTJscInstance : public JSEngineInstance {
public:
RCTJscInstance();

std::unique_ptr<jsi::Runtime> createJSRuntime() noexcept override;
std::unique_ptr<jsi::Runtime> createJSRuntime(
std::shared_ptr<MessageQueueThread> msgQueueThread) noexcept override;

~RCTJscInstance(){};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@

RCTJscInstance::RCTJscInstance() {}

std::unique_ptr<jsi::Runtime> RCTJscInstance::createJSRuntime() noexcept
std::unique_ptr<jsi::Runtime> RCTJscInstance::createJSRuntime(
std::shared_ptr<MessageQueueThread> msgQueueThread) noexcept
{
return jsc::makeJSCRuntime();
}
Expand Down

0 comments on commit fe4dab9

Please sign in to comment.