Skip to content

Commit 4013ab3

Browse files
Fix the data race in NetworkCurl (#1595)
Fix the RequestHandle initialization; have to be protected with a mutex Fix the condition in cancel event handling, to avoid canceling wrong requests Relates-To: DATASDK-69 Signed-off-by: Mykhailo Kuchma <ext-mykhailo.kuchma@here.com>
1 parent 884a201 commit 4013ab3

File tree

2 files changed

+23
-16
lines changed

2 files changed

+23
-16
lines changed

olp-cpp-sdk-core/src/http/curl/NetworkCurl.cpp

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -678,21 +678,28 @@ ErrorCode NetworkCurl::SendImplementation(
678678

679679
const auto& config = request.GetSettings();
680680

681-
RequestHandle* handle = InitRequestHandle();
681+
RequestHandle* handle = [&] {
682+
std::lock_guard<std::mutex> lock(event_mutex_);
683+
684+
auto* request_handle = InitRequestHandleUnsafe();
685+
686+
if (request_handle) {
687+
request_handle->id = id;
688+
request_handle->out_completion_callback = std::move(callback);
689+
request_handle->out_header_callback = std::move(header_callback);
690+
request_handle->out_data_callback = std::move(data_callback);
691+
request_handle->out_data_stream = payload;
692+
request_handle->request_body = request.GetBody();
693+
request_handle->request_headers = SetupHeaders(request.GetHeaders());
694+
}
695+
696+
return request_handle;
697+
}();
698+
682699
if (!handle) {
683700
return ErrorCode::NETWORK_OVERLOAD_ERROR;
684701
}
685702

686-
handle->id = id;
687-
688-
// Set request output callbacks
689-
handle->out_completion_callback = std::move(callback);
690-
handle->out_header_callback = std::move(header_callback);
691-
handle->out_data_callback = std::move(data_callback);
692-
handle->out_data_stream = payload;
693-
handle->request_body = request.GetBody();
694-
handle->request_headers = SetupHeaders(request.GetHeaders());
695-
696703
OLP_SDK_LOG_DEBUG(kLogTag,
697704
"Send request with url="
698705
<< utils::CensorCredentialsInUrl(request.GetUrl())
@@ -868,9 +875,7 @@ void NetworkCurl::AddEvent(EventInfo::Type type, RequestHandle* handle) {
868875
#endif
869876
}
870877

871-
NetworkCurl::RequestHandle* NetworkCurl::InitRequestHandle() {
872-
std::lock_guard<std::mutex> lock(event_mutex_);
873-
878+
NetworkCurl::RequestHandle* NetworkCurl::InitRequestHandleUnsafe() {
874879
const auto unused_handle_it =
875880
std::find_if(handles_.begin(), handles_.end(),
876881
[](const RequestHandle& request_handle) {
@@ -1162,7 +1167,8 @@ void NetworkCurl::Run() {
11621167
msgs.push_back(curl_handle);
11631168
}
11641169
}
1165-
} else {
1170+
} else if (event.type == EventInfo::Type::CANCEL_EVENT &&
1171+
request_handle->is_cancelled) {
11661172
// The Request was canceled, remove it from curl
11671173
auto code = curl_multi_remove_handle(curl_, curl_handle);
11681174
if (code != CURLM_OK) {

olp-cpp-sdk-core/src/http/curl/NetworkCurl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,10 +239,11 @@ class NetworkCurl : public Network,
239239

240240
/**
241241
* @brief Allocate new handle RequestHandle.
242+
* @note Must be protected by event_mutex_
242243
*
243244
* @return Pointer to the allocated RequestHandle.
244245
*/
245-
RequestHandle* InitRequestHandle();
246+
RequestHandle* InitRequestHandleUnsafe();
246247

247248
/**
248249
* @brief Reset the handle after network request is done.

0 commit comments

Comments
 (0)