Skip to content

Commit 6eb456b

Browse files
huntiefacebook-github-bot
authored andcommitted
Fix data race condition in NetworkHandler (#53788)
Summary: Pull Request resolved: #53788 Fix thread safety issue due to member variable in `NetworkHandler` singleton without mutex. Also intend to un-singleton this class in the imminent future. Changelog: [Internal] Reviewed By: hoxyq Differential Revision: D82460574 fbshipit-source-id: c0c614f8f1bb5ffba22872ae5717fbd2ca01f2e9
1 parent 27630d3 commit 6eb456b

File tree

2 files changed

+26
-19
lines changed

2 files changed

+26
-19
lines changed

packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ bool NetworkHandler::disable() {
5656
}
5757

5858
enabled_.store(false, std::memory_order_release);
59-
requestBodyBuffer_.clear();
59+
responseBodyBuffer_.clear();
6060
return true;
6161
}
6262

@@ -113,7 +113,10 @@ void NetworkHandler::onResponseReceived(
113113
}
114114

115115
auto resourceType = cdp::network::resourceTypeFromMimeType(response.mimeType);
116-
resourceTypeMap_.emplace(requestId, resourceType);
116+
{
117+
std::lock_guard<std::mutex> lock(resourceTypeMapMutex_);
118+
resourceTypeMap_.emplace(requestId, resourceType);
119+
}
117120

118121
auto params = cdp::network::ResponseReceivedParams{
119122
.requestId = requestId,
@@ -166,37 +169,40 @@ void NetworkHandler::onLoadingFinished(
166169

167170
void NetworkHandler::onLoadingFailed(
168171
const std::string& requestId,
169-
bool cancelled) const {
172+
bool cancelled) {
170173
if (!isEnabledNoSync()) {
171174
return;
172175
}
173176

174-
auto params = cdp::network::LoadingFailedParams{
175-
.requestId = requestId,
176-
.timestamp = getCurrentUnixTimestampSeconds(),
177-
.type = resourceTypeMap_.find(requestId) != resourceTypeMap_.end()
178-
? resourceTypeMap_.at(requestId)
179-
: "Other",
180-
.errorText = cancelled ? "net::ERR_ABORTED" : "net::ERR_FAILED",
181-
.canceled = cancelled,
182-
};
183-
184-
frontendChannel_(
185-
cdp::jsonNotification("Network.loadingFailed", params.toDynamic()));
177+
{
178+
std::lock_guard<std::mutex> lock(resourceTypeMapMutex_);
179+
auto params = cdp::network::LoadingFailedParams{
180+
.requestId = requestId,
181+
.timestamp = getCurrentUnixTimestampSeconds(),
182+
.type = resourceTypeMap_.find(requestId) != resourceTypeMap_.end()
183+
? resourceTypeMap_.at(requestId)
184+
: "Other",
185+
.errorText = cancelled ? "net::ERR_ABORTED" : "net::ERR_FAILED",
186+
.canceled = cancelled,
187+
};
188+
189+
frontendChannel_(
190+
cdp::jsonNotification("Network.loadingFailed", params.toDynamic()));
191+
}
186192
}
187193

188194
void NetworkHandler::storeResponseBody(
189195
const std::string& requestId,
190196
std::string_view body,
191197
bool base64Encoded) {
192198
std::lock_guard<std::mutex> lock(requestBodyMutex_);
193-
requestBodyBuffer_.put(requestId, body, base64Encoded);
199+
responseBodyBuffer_.put(requestId, body, base64Encoded);
194200
}
195201

196202
std::optional<std::tuple<std::string, bool>> NetworkHandler::getResponseBody(
197203
const std::string& requestId) {
198204
std::lock_guard<std::mutex> lock(requestBodyMutex_);
199-
auto responseBody = requestBodyBuffer_.get(requestId);
205+
auto responseBody = responseBodyBuffer_.get(requestId);
200206

201207
if (responseBody == nullptr) {
202208
return std::nullopt;

packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class NetworkHandler {
100100
/**
101101
* @cdp Network.loadingFailed
102102
*/
103-
void onLoadingFailed(const std::string& requestId, bool cancelled) const;
103+
void onLoadingFailed(const std::string& requestId, bool cancelled);
104104

105105
/**
106106
* Store the fetched response body for a text or image network response.
@@ -139,8 +139,9 @@ class NetworkHandler {
139139
FrontendChannel frontendChannel_;
140140

141141
std::map<std::string, std::string> resourceTypeMap_{};
142+
std::mutex resourceTypeMapMutex_{};
142143

143-
BoundedRequestBuffer requestBodyBuffer_{};
144+
BoundedRequestBuffer responseBodyBuffer_{};
144145
std::mutex requestBodyMutex_;
145146
};
146147

0 commit comments

Comments
 (0)