From 6eb456bc4e6bcdf2f9f9b41c4b8265b9bee3af54 Mon Sep 17 00:00:00 2001 From: Alex Hunt Date: Tue, 16 Sep 2025 03:03:06 -0700 Subject: [PATCH] Fix data race condition in NetworkHandler (#53788) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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 --- .../network/NetworkHandler.cpp | 38 +++++++++++-------- .../network/NetworkHandler.h | 5 ++- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp b/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp index 80ed5bb23b1..5e7c1b558a2 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp @@ -56,7 +56,7 @@ bool NetworkHandler::disable() { } enabled_.store(false, std::memory_order_release); - requestBodyBuffer_.clear(); + responseBodyBuffer_.clear(); return true; } @@ -113,7 +113,10 @@ void NetworkHandler::onResponseReceived( } auto resourceType = cdp::network::resourceTypeFromMimeType(response.mimeType); - resourceTypeMap_.emplace(requestId, resourceType); + { + std::lock_guard lock(resourceTypeMapMutex_); + resourceTypeMap_.emplace(requestId, resourceType); + } auto params = cdp::network::ResponseReceivedParams{ .requestId = requestId, @@ -166,23 +169,26 @@ void NetworkHandler::onLoadingFinished( void NetworkHandler::onLoadingFailed( const std::string& requestId, - bool cancelled) const { + bool cancelled) { if (!isEnabledNoSync()) { return; } - auto params = cdp::network::LoadingFailedParams{ - .requestId = requestId, - .timestamp = getCurrentUnixTimestampSeconds(), - .type = resourceTypeMap_.find(requestId) != resourceTypeMap_.end() - ? resourceTypeMap_.at(requestId) - : "Other", - .errorText = cancelled ? "net::ERR_ABORTED" : "net::ERR_FAILED", - .canceled = cancelled, - }; + { + std::lock_guard lock(resourceTypeMapMutex_); + auto params = cdp::network::LoadingFailedParams{ + .requestId = requestId, + .timestamp = getCurrentUnixTimestampSeconds(), + .type = resourceTypeMap_.find(requestId) != resourceTypeMap_.end() + ? resourceTypeMap_.at(requestId) + : "Other", + .errorText = cancelled ? "net::ERR_ABORTED" : "net::ERR_FAILED", + .canceled = cancelled, + }; - frontendChannel_( - cdp::jsonNotification("Network.loadingFailed", params.toDynamic())); + frontendChannel_( + cdp::jsonNotification("Network.loadingFailed", params.toDynamic())); + } } void NetworkHandler::storeResponseBody( @@ -190,13 +196,13 @@ void NetworkHandler::storeResponseBody( std::string_view body, bool base64Encoded) { std::lock_guard lock(requestBodyMutex_); - requestBodyBuffer_.put(requestId, body, base64Encoded); + responseBodyBuffer_.put(requestId, body, base64Encoded); } std::optional> NetworkHandler::getResponseBody( const std::string& requestId) { std::lock_guard lock(requestBodyMutex_); - auto responseBody = requestBodyBuffer_.get(requestId); + auto responseBody = responseBodyBuffer_.get(requestId); if (responseBody == nullptr) { return std::nullopt; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.h b/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.h index cb38d1b7628..b3c51b83ce4 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.h @@ -100,7 +100,7 @@ class NetworkHandler { /** * @cdp Network.loadingFailed */ - void onLoadingFailed(const std::string& requestId, bool cancelled) const; + void onLoadingFailed(const std::string& requestId, bool cancelled); /** * Store the fetched response body for a text or image network response. @@ -139,8 +139,9 @@ class NetworkHandler { FrontendChannel frontendChannel_; std::map resourceTypeMap_{}; + std::mutex resourceTypeMapMutex_{}; - BoundedRequestBuffer requestBodyBuffer_{}; + BoundedRequestBuffer responseBodyBuffer_{}; std::mutex requestBodyMutex_; };