Skip to content

Commit

Permalink
Porting down NetworkingModule fixes (#4188)
Browse files Browse the repository at this point in the history
* Fix concurrency issue in NetworkingModule (#4179)

* Add locking around access to m_requests

* Change files

* add locking around AbortRequest as well

* fix formatting

* lock needs local variable

* Remove useIncrementalUpdates assert from Networking module (#4116)

* Remove useIncrementalUpdates assert from Networking module

* Change files

* add [[maybe_unused]]

Co-authored-by: Marlene Cota <marlenecota@gmail.com>
  • Loading branch information
lamxdoan and marlenecota authored Feb 26, 2020
1 parent 904d6c6 commit 4c97927
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"type": "prerelease",
"comment": "Remove useIncrementalUpdates assert from Networking module",
"packageName": "react-native-windows",
"email": "mcota@microsoft.com",
"commit": "845055fb8bac6a1df37fc863ea505220744432d1",
"dependentChangeType": "patch",
"date": "2020-02-19T01:30:22.945Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"type": "prerelease",
"comment": "Fix concurrency issue in NetworkingModule",
"packageName": "react-native-windows",
"email": "lamdoan@microsoft.com",
"commit": "9a4348df3b8177f0bf4bc451bbc1d93df5215dc4",
"dependentChangeType": "patch",
"date": "2020-02-25T19:16:19.779Z"
}
21 changes: 15 additions & 6 deletions vnext/ReactUWP/Modules/NetworkingModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ class NetworkingModule::NetworkingHelper : public std::enable_shared_from_this<N
winrt::Windows::Foundation::IAsyncOperationWithProgress<
winrt::Windows::Web::Http::HttpResponseMessage,
winrt::Windows::Web::Http::HttpProgress> completion) {
std::scoped_lock lock(m_mutex);
m_requests[requestId] = completion;
}
void RemoveRequest(int64_t requestId) {
std::scoped_lock lock(m_mutex);
m_requests.erase(requestId);
}

Expand All @@ -87,6 +89,7 @@ class NetworkingModule::NetworkingHelper : public std::enable_shared_from_this<N
winrt::Windows::Web::Http::HttpResponseMessage,
winrt::Windows::Web::Http::HttpProgress>>
m_requests;
std::mutex m_mutex;
static std::int64_t s_lastRequestId;
};

Expand Down Expand Up @@ -235,14 +238,13 @@ void NetworkingModule::NetworkingHelper::SendRequest(
const folly::dynamic &headers,
folly::dynamic bodyData,
const std::string &responseType,
bool useIncrementalUpdates,
[[maybe_unused]] bool useIncrementalUpdates,
int64_t timeout,
Callback cb) noexcept {
int64_t requestId = ++s_lastRequestId;

// Enforce supported args
assert(responseType == "text" || responseType == "base64");
assert(!useIncrementalUpdates);

// Callback with the requestId
cb({requestId});
Expand Down Expand Up @@ -337,11 +339,18 @@ void NetworkingModule::NetworkingHelper::SendRequest(
}

void NetworkingModule::NetworkingHelper::AbortRequest(int64_t requestId) noexcept {
auto iter = m_requests.find(requestId);
if (iter == end(m_requests))
return;
winrt::Windows::Foundation::IAsyncOperationWithProgress<
winrt::Windows::Web::Http::HttpResponseMessage,
winrt::Windows::Web::Http::HttpProgress>
httpRequest(nullptr);
{
std::scoped_lock lock(m_mutex);
auto iter = m_requests.find(requestId);
if (iter == end(m_requests))
return;
httpRequest = iter->second;
}

auto httpRequest = iter->second;
try {
httpRequest.Cancel();
} catch (...) {
Expand Down

0 comments on commit 4c97927

Please sign in to comment.