Skip to content

Commit 8bbd422

Browse files
Abbondanzometa-codesync[bot]
authored andcommitted
Synchronize native Fantom stub queue (#59015)
Summary: Pull Request resolved: #59015 Fantom’s native `StubQueue` can be written by multiple threads while it is being drained, but its queue access was unsynchronized. Protect queue storage with a mutex while keeping callback invocation and destruction outside the lock, so callbacks can reenter the queue. This is a tester-only change. The single-consumer and queue-lifetime requirements are unchanged; startup readiness and shared runtime teardown are out of scope. Changelog: [Internal] - Synchronize native Fantom stub queue access ___ landed-with-radar-review Differential Revision: D124675719 fbshipit-source-id: 701bcce6702624584f6f18ad2ed94ca3227aa68d
1 parent 9f5a12c commit 8bbd422

3 files changed

Lines changed: 220 additions & 13 deletions

File tree

‎private/react-native-fantom/tester/src/stubs/StubQueue.cpp‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010
namespace facebook::react {
1111

1212
void StubQueue::runOnQueue(std::function<void()>&& func) {
13-
callbackQueue_.push(func);
13+
auto callback = std::make_unique<std::function<void()>>(std::move(func));
14+
{
15+
std::lock_guard lock(mutex_);
16+
callbackQueue_.push(std::move(callback));
17+
}
1418
}
1519

1620
void StubQueue::runOnQueueSync(std::function<void()>&& runnable) {
@@ -23,23 +27,29 @@ void StubQueue::quitSynchronous() {
2327
}
2428

2529
void StubQueue::flush() {
26-
while (!callbackQueue_.empty()) {
27-
tick();
30+
while (tick()) {
2831
}
2932
}
3033

3134
bool StubQueue::hasPendingCallbacks() {
35+
std::lock_guard lock(mutex_);
3236
return !callbackQueue_.empty();
3337
}
3438

35-
void StubQueue::tick() {
36-
if (!callbackQueue_.empty()) {
37-
auto callback = callbackQueue_.front();
38-
callbackQueue_.pop();
39-
if (callback) {
40-
callback();
39+
bool StubQueue::tick() {
40+
std::unique_ptr<std::function<void()>> callback;
41+
{
42+
std::lock_guard lock(mutex_);
43+
if (callbackQueue_.empty()) {
44+
return false;
4145
}
46+
callback = std::move(callbackQueue_.front());
47+
callbackQueue_.pop();
48+
}
49+
if (*callback) {
50+
(*callback)();
4251
}
52+
return true;
4353
}
4454

4555
} // namespace facebook::react

‎private/react-native-fantom/tester/src/stubs/StubQueue.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
#pragma once
99

1010
#include <cxxreact/MessageQueueThread.h>
11-
#include <glog/logging.h>
12-
#include <condition_variable>
11+
#include <memory>
12+
#include <mutex>
1313
#include <queue>
1414

1515
namespace facebook::react {
@@ -24,12 +24,14 @@ class StubQueue : public MessageQueueThread {
2424

2525
bool hasPendingCallbacks();
2626

27+
// Draining belongs to the single consumer thread.
2728
void flush();
2829

2930
private:
30-
void tick();
31+
bool tick();
3132

32-
std::queue<std::function<void()>> callbackQueue_;
33+
std::mutex mutex_;
34+
std::queue<std::unique_ptr<std::function<void()>>> callbackQueue_;
3335
};
3436

3537
} // namespace facebook::react
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
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 <gtest/gtest.h>
9+
#include <react/fantom/StubQueue.h>
10+
#include <atomic>
11+
#include <chrono>
12+
#include <future>
13+
#include <stdexcept>
14+
#include <thread>
15+
#include <vector>
16+
17+
namespace facebook::react {
18+
19+
TEST(StubQueueTest, FlushPreservesOrderAndAllowsReentrantEnqueue) {
20+
StubQueue queue;
21+
std::vector<int> callbacks;
22+
queue.runOnQueue([&] {
23+
callbacks.push_back(1);
24+
queue.runOnQueue([&] { callbacks.push_back(3); });
25+
queue.flush();
26+
});
27+
queue.runOnQueue([&] { callbacks.push_back(2); });
28+
29+
queue.flush();
30+
31+
EXPECT_EQ(callbacks, (std::vector<int>{1, 2, 3}));
32+
EXPECT_FALSE(queue.hasPendingCallbacks());
33+
}
34+
35+
TEST(StubQueueTest, FlushDoesNotCopySmallNoexceptCallable) {
36+
struct State {
37+
int copies{0};
38+
int calls{0};
39+
};
40+
struct Callback {
41+
explicit Callback(std::shared_ptr<State> state) : state(std::move(state)) {}
42+
Callback(const Callback& other) noexcept : state(other.state) {
43+
++state->copies;
44+
}
45+
Callback& operator=(const Callback&) = default;
46+
void operator()() const {
47+
++state->calls;
48+
}
49+
std::shared_ptr<State> state;
50+
};
51+
StubQueue queue;
52+
auto state = std::make_shared<State>();
53+
queue.runOnQueue(Callback(state));
54+
state->copies = 0;
55+
56+
queue.flush();
57+
58+
EXPECT_EQ(state->calls, 1);
59+
EXPECT_EQ(state->copies, 0);
60+
}
61+
62+
TEST(StubQueueTest, CallableTransfersAllowReentrantCopyAndDestruction) {
63+
std::promise<int> result;
64+
auto completed = result.get_future();
65+
std::thread worker([result = std::move(result)]() mutable {
66+
StubQueue queue;
67+
struct State {
68+
StubQueue* queue{nullptr};
69+
bool armed{false};
70+
int calls{0};
71+
};
72+
struct Callback {
73+
explicit Callback(std::shared_ptr<State> state)
74+
: state(std::move(state)) {}
75+
Callback(const Callback& other) noexcept : state(other.state) {
76+
probe();
77+
}
78+
Callback(Callback&&) noexcept = default;
79+
Callback& operator=(const Callback&) = default;
80+
Callback& operator=(Callback&&) noexcept = delete;
81+
~Callback() {
82+
probe();
83+
}
84+
void probe() const noexcept {
85+
if (state && state->armed) {
86+
(void)state->queue->hasPendingCallbacks();
87+
}
88+
}
89+
void operator()() const {
90+
++state->calls;
91+
}
92+
std::shared_ptr<State> state;
93+
};
94+
auto state = std::make_shared<State>();
95+
state->queue = &queue;
96+
std::function<void()> callback = Callback(state);
97+
state->armed = true;
98+
queue.runOnQueue(std::move(callback));
99+
callback = nullptr;
100+
queue.flush();
101+
result.set_value(state->calls);
102+
});
103+
auto status = completed.wait_for(std::chrono::seconds(5));
104+
if (status == std::future_status::ready) {
105+
worker.join();
106+
} else {
107+
// NOLINTNEXTLINE(facebook-hte-BadCall-detach): failure-only private state.
108+
worker.detach();
109+
}
110+
ASSERT_EQ(status, std::future_status::ready);
111+
EXPECT_EQ(completed.get(), 1);
112+
}
113+
114+
TEST(StubQueueTest, FlushSkipsEmptyCallbacks) {
115+
StubQueue queue;
116+
int calls = 0;
117+
queue.runOnQueue({});
118+
queue.runOnQueue([&] { ++calls; });
119+
120+
queue.flush();
121+
122+
EXPECT_EQ(calls, 1);
123+
EXPECT_FALSE(queue.hasPendingCallbacks());
124+
}
125+
126+
TEST(StubQueueTest, ConcurrentProducersExecuteExactlyOnceInProducerOrder) {
127+
StubQueue queue;
128+
constexpr int producerCount = 8;
129+
constexpr int callbacksPerProducer = 10000;
130+
const auto consumer = std::this_thread::get_id();
131+
std::promise<void> start;
132+
auto started = start.get_future().share();
133+
std::atomic<int> completedProducers{0};
134+
int executed = 0;
135+
std::vector<int> callbacks(producerCount, 0);
136+
std::vector<std::thread> producers;
137+
producers.reserve(producerCount);
138+
for (int producer = 0; producer < producerCount; ++producer) {
139+
producers.emplace_back([&, producer] {
140+
started.wait();
141+
for (int index = 0; index < callbacksPerProducer; ++index) {
142+
queue.runOnQueue([&, producer, index] {
143+
EXPECT_EQ(std::this_thread::get_id(), consumer);
144+
EXPECT_EQ(callbacks[producer], index);
145+
++callbacks[producer];
146+
++executed;
147+
});
148+
}
149+
++completedProducers;
150+
});
151+
}
152+
153+
start.set_value();
154+
while (completedProducers.load() != producerCount) {
155+
queue.flush();
156+
std::this_thread::yield();
157+
}
158+
for (auto& producer : producers) {
159+
producer.join();
160+
}
161+
queue.flush();
162+
163+
EXPECT_EQ(executed, producerCount * callbacksPerProducer);
164+
EXPECT_EQ(callbacks, std::vector<int>(producerCount, callbacksPerProducer));
165+
EXPECT_FALSE(queue.hasPendingCallbacks());
166+
}
167+
168+
TEST(StubQueueTest, SynchronousWorkRunsAfterPendingCallbacks) {
169+
StubQueue queue;
170+
std::vector<int> callbacks;
171+
queue.runOnQueue([&] { callbacks.push_back(1); });
172+
173+
queue.runOnQueueSync([&] { callbacks.push_back(2); });
174+
175+
EXPECT_EQ(callbacks, (std::vector<int>{1, 2}));
176+
}
177+
178+
TEST(StubQueueTest, ThrowingCallbackLeavesLaterWorkDrainable) {
179+
StubQueue queue;
180+
std::vector<int> callbacks;
181+
queue.runOnQueue([&] {
182+
callbacks.push_back(1);
183+
throw std::runtime_error("queued failure");
184+
});
185+
queue.runOnQueue([&] { callbacks.push_back(2); });
186+
187+
EXPECT_THROW(queue.flush(), std::runtime_error);
188+
EXPECT_TRUE(queue.hasPendingCallbacks());
189+
queue.flush();
190+
191+
EXPECT_EQ(callbacks, (std::vector<int>{1, 2}));
192+
EXPECT_FALSE(queue.hasPendingCallbacks());
193+
}
194+
195+
} // namespace facebook::react

0 commit comments

Comments
 (0)