Skip to content

Commit b043c27

Browse files
javachemeta-codesync[bot]
authored andcommitted
Add unit test for RuntimeExecutor post-shutdown safety (#56995)
Summary: Pull Request resolved: #56995 The `RuntimeExecutor` returned by `ReactInstance::getBufferedRuntimeExecutor()` is designed to be safe to hold past the lifetime of the `ReactInstance` itself — callers may schedule work on it after shutdown without crashing, and any pending callbacks must be dropped rather than surfaced on the JS queue. This contract relies on `weak_ptr` captures inside the executor closure and inside the base closure in the `ReactInstance` constructor. Today nothing tests it directly — coverage is incidental through `ReactInstanceTest` happy-path cases — so a future refactor that swaps a `weak_ptr` capture for a `shared_ptr` (keeping the runtime alive) or a raw pointer (introducing UAF) could silently regress. Adds three focused regression tests covering: - Calling the held executor after `ReactInstance` destruction drops the callback (no queuing, no execution). - Repeated post-shutdown invocations remain memory-safe. - Work buffered in `BufferedRuntimeExecutor` before shutdown is dropped, never surfaced on the JS queue. The new test file lands under the existing `tests` target's `glob(["tests/**/*.cpp"])`, so no BUCK changes are needed. Note: this test deliberately targets the buffered API only. `getUnbufferedRuntimeExecutor()` captures `runtimeScheduler_.get()` as a raw pointer and is not safe to hold past shutdown by design — a separate concern, out of scope here. Changelog: [Internal] ___ Differential Revision: D106783789 fbshipit-source-id: d28f07d9c79daf8596415686f17e6352c34dfb29
1 parent 707e366 commit b043c27

2 files changed

Lines changed: 152 additions & 3 deletions

File tree

packages/react-native/ReactCommon/react/runtime/tests/cxx/ReactInstanceTest.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ namespace facebook::react {
2626

2727
class MockTimerRegistry : public PlatformTimerRegistry {
2828
public:
29-
MOCK_METHOD2(createTimer, void(uint32_t, double));
30-
MOCK_METHOD2(createRecurringTimer, void(uint32_t, double));
31-
MOCK_METHOD1(deleteTimer, void(uint32_t));
29+
MOCK_METHOD(void, createTimer, (uint32_t, double), (override));
30+
MOCK_METHOD(void, createRecurringTimer, (uint32_t, double), (override));
31+
MOCK_METHOD(void, deleteTimer, (uint32_t), (override));
3232
};
3333

3434
class MockMessageQueueThread : public MessageQueueThread {
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#include <memory>
9+
#include <queue>
10+
#include <utility>
11+
12+
#include <gmock/gmock.h>
13+
#include <gtest/gtest.h>
14+
15+
#include <ReactCommon/RuntimeExecutor.h>
16+
#include <hermes/hermes.h>
17+
#include <jserrorhandler/JsErrorHandler.h>
18+
#include <jsi/jsi.h>
19+
#include <react/runtime/ReactInstance.h>
20+
21+
namespace facebook::react {
22+
23+
namespace {
24+
25+
class MockTimerRegistry : public PlatformTimerRegistry {
26+
public:
27+
MOCK_METHOD(void, createTimer, (uint32_t, double), (override));
28+
MOCK_METHOD(void, createRecurringTimer, (uint32_t, double), (override));
29+
MOCK_METHOD(void, deleteTimer, (uint32_t), (override));
30+
};
31+
32+
class MockMessageQueueThread : public MessageQueueThread {
33+
public:
34+
void runOnQueue(std::function<void()>&& func) override {
35+
callbackQueue_.push(std::move(func));
36+
}
37+
38+
void runOnQueueSync(std::function<void()>&& /*unused*/) override {}
39+
void quitSynchronous() override {}
40+
41+
void tick() {
42+
while (!callbackQueue_.empty()) {
43+
auto callback = std::move(callbackQueue_.front());
44+
callbackQueue_.pop();
45+
callback();
46+
}
47+
}
48+
49+
size_t size() const {
50+
return callbackQueue_.size();
51+
}
52+
53+
private:
54+
std::queue<std::function<void()>> callbackQueue_;
55+
};
56+
57+
} // namespace
58+
59+
class RuntimeExecutorShutdownTest : public ::testing::Test {
60+
protected:
61+
void SetUp() override {
62+
auto runtime =
63+
std::make_unique<JSIRuntimeHolder>(hermes::makeHermesRuntime());
64+
messageQueueThread_ = std::make_shared<MockMessageQueueThread>();
65+
auto mockRegistry = std::make_unique<MockTimerRegistry>();
66+
auto timerManager = std::make_shared<TimerManager>(std::move(mockRegistry));
67+
auto onJsError =
68+
[](jsi::Runtime& /*runtime*/,
69+
const JsErrorHandler::ProcessedError& /*error*/) noexcept {};
70+
71+
instance_ = std::make_unique<ReactInstance>(
72+
std::move(runtime),
73+
messageQueueThread_,
74+
timerManager,
75+
std::move(onJsError));
76+
}
77+
78+
std::shared_ptr<MockMessageQueueThread> messageQueueThread_;
79+
std::unique_ptr<ReactInstance> instance_;
80+
};
81+
82+
// Calling a held RuntimeExecutor after ReactInstance has been destroyed
83+
// must silently drop the callback. Use a shared_ptr captured into the
84+
// callback to also prove the callback itself is destroyed (not retained
85+
// somewhere indefinitely).
86+
TEST_F(
87+
RuntimeExecutorShutdownTest,
88+
heldExecutorCallAfterShutdownDropsCallback) {
89+
RuntimeExecutor executor = instance_->getBufferedRuntimeExecutor();
90+
91+
instance_.reset();
92+
93+
auto tracker = std::make_shared<int>(0);
94+
std::weak_ptr<int> weakTracker = tracker;
95+
executor([tracker = std::move(tracker)](jsi::Runtime& /*runtime*/) {
96+
FAIL() << "Callback should never run after shutdown";
97+
});
98+
99+
EXPECT_EQ(messageQueueThread_->size(), 0u);
100+
EXPECT_TRUE(weakTracker.expired())
101+
<< "Rejected callback (and its captures) should be destroyed";
102+
messageQueueThread_->tick();
103+
}
104+
105+
// Repeated post-shutdown invocations must remain memory-safe. This guards
106+
// against future refactors that swap the weak_ptr capture for a raw pointer
107+
// or shared_ptr.
108+
TEST_F(RuntimeExecutorShutdownTest, heldExecutorCallAfterShutdownDoesNotCrash) {
109+
RuntimeExecutor executor = instance_->getBufferedRuntimeExecutor();
110+
111+
instance_.reset();
112+
113+
for (int i = 0; i < 100; ++i) {
114+
executor([](jsi::Runtime& /*runtime*/) {
115+
FAIL() << "Callback should never run after shutdown";
116+
});
117+
}
118+
messageQueueThread_->tick();
119+
}
120+
121+
// Work scheduled through the buffered executor sits in the internal buffer
122+
// until flush() (which only happens via loadScript). If the ReactInstance
123+
// is destroyed before flush, the buffered work must be dropped — never
124+
// surfaced on the JS queue and never retained past instance destruction.
125+
// A shared_ptr captured into the callback, watched via weak_ptr from
126+
// outside, proves the buffered queue actually releases its contents.
127+
TEST_F(
128+
RuntimeExecutorShutdownTest,
129+
pendingCallbackShutdownBeforeTickDropsCallback) {
130+
RuntimeExecutor executor = instance_->getBufferedRuntimeExecutor();
131+
132+
auto tracker = std::make_shared<int>(0);
133+
std::weak_ptr<int> weakTracker = tracker;
134+
executor([tracker = std::move(tracker)](jsi::Runtime& /*runtime*/) {
135+
FAIL() << "Buffered callback should never run after shutdown";
136+
});
137+
138+
EXPECT_EQ(messageQueueThread_->size(), 0u);
139+
EXPECT_FALSE(weakTracker.expired())
140+
<< "Callback should still be alive in BufferedRuntimeExecutor's queue";
141+
142+
instance_.reset();
143+
144+
EXPECT_TRUE(weakTracker.expired())
145+
<< "Buffered callback should be destroyed when ReactInstance is";
146+
messageQueueThread_->tick();
147+
}
148+
149+
} // namespace facebook::react

0 commit comments

Comments
 (0)