Skip to content

Commit 5f74c47

Browse files
exposightVasyl Bilous
authored andcommitted
Fix NetworkCurl race on pipe close
ThreadSanitizer detected a data race on the pipe member between deinitializing thread making notification and the worker thread doing teardown. Relates-To: HERESDK-4984, OCMAM-772 Signed-off-by: Vasyl Bilous <ext-vasyl.bilous@here.com>
1 parent 3496dd4 commit 5f74c47

1 file changed

Lines changed: 12 additions & 12 deletions

File tree

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -575,20 +575,20 @@ void NetworkCurl::Deinitialize() {
575575
std::lock_guard<std::mutex> init_lock(init_mutex_);
576576

577577
// Stop worker thread
578-
if (!IsStarted()) {
578+
auto expected_state = WorkerState::STARTED;
579+
if (!impl_->state_->compare_exchange_strong(expected_state,
580+
WorkerState::STOPPING)) {
579581
OLP_SDK_LOG_DEBUG(kLogTag, "Already deinitialized, this=" << this);
580582
return;
581583
}
582584

583585
OLP_SDK_LOG_TRACE(kLogTag, "Deinitialize NetworkCurl, this=" << this);
584586

585-
{
586-
std::lock_guard<std::mutex> lock(impl_->event_mutex_);
587-
*impl_->state_ = WorkerState::STOPPING;
588-
}
589-
590587
if (thread_.get_id() != std::this_thread::get_id()) {
591-
impl_->NotifyEvent();
588+
{
589+
std::lock_guard<std::mutex> lock(impl_->event_mutex_);
590+
impl_->NotifyEvent();
591+
}
592592
thread_.join();
593593
} else {
594594
// We are trying to stop the very thread we are in. This could happen if the
@@ -625,6 +625,11 @@ void NetworkCurl::Impl::Teardown() {
625625
// cURL teardown
626626
curl_multi_cleanup(curl_);
627627
curl_ = nullptr;
628+
629+
#if (defined OLP_SDK_NETWORK_HAS_PIPE) || (defined OLP_SDK_NETWORK_HAS_PIPE2)
630+
close(pipe_[0]);
631+
close(pipe_[1]);
632+
#endif
628633
}
629634

630635
// Handle completed messages
@@ -636,11 +641,6 @@ void NetworkCurl::Impl::Teardown() {
636641
.WithError("Offline: network is deinitialized"));
637642
}
638643
}
639-
640-
#if (defined OLP_SDK_NETWORK_HAS_PIPE) || (defined OLP_SDK_NETWORK_HAS_PIPE2)
641-
close(pipe_[0]);
642-
close(pipe_[1]);
643-
#endif
644644
}
645645

646646
bool NetworkCurl::IsStarted() const { return impl_->IsStarted(); }

0 commit comments

Comments
 (0)