From 04407288cadbc913cc59fb875dfca68bdb7ba3af Mon Sep 17 00:00:00 2001 From: owentou Date: Tue, 9 Aug 2022 22:50:18 +0800 Subject: [PATCH] Fix infinitely waiting when shutdown with more than one running http sessions. Signed-off-by: owentou --- exporters/otlp/src/otlp_http_client.cc | 16 ++++++++++++---- ext/src/http/client/curl/http_client_curl.cc | 13 +++++++++---- ext/src/http/client/curl/http_operation_curl.cc | 2 ++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/exporters/otlp/src/otlp_http_client.cc b/exporters/otlp/src/otlp_http_client.cc index 40234be46b..58c5d78e66 100644 --- a/exporters/otlp/src/otlp_http_client.cc +++ b/exporters/otlp/src/otlp_http_client.cc @@ -668,8 +668,12 @@ OtlpHttpClient::~OtlpHttpClient() } } // When changes of running_sessions_ and notify_one/notify_all happen between predicate - // checking and waiting, we should not wait forever. - session_waker_.wait_for(lock, options_.timeout); + // checking and waiting, we should not wait forever. We should cleanup gc sessions here as soon + // as possible to call FinishSession() and cleanup resources. + if (std::cv_status::timeout == session_waker_.wait_for(lock, options_.timeout)) + { + cleanupGCSessions(); + } } // And then remove all session datas @@ -781,8 +785,12 @@ bool OtlpHttpClient::ForceFlush(std::chrono::microseconds timeout) noexcept } } // When changes of running_sessions_ and notify_one/notify_all happen between predicate - // checking and waiting, we should not wait forever. - session_waker_.wait_for(lock, options_.timeout); + // checking and waiting, we should not wait forever.We should cleanup gc sessions here as soon + // as possible to call FinishSession() and cleanup resources. + if (std::cv_status::timeout == session_waker_.wait_for(lock, options_.timeout)) + { + cleanupGCSessions(); + } } return true; } diff --git a/ext/src/http/client/curl/http_client_curl.cc b/ext/src/http/client/curl/http_client_curl.cc index 4dcdb4bccd..3ba0133633 100644 --- a/ext/src/http/client/curl/http_client_curl.cc +++ b/ext/src/http/client/curl/http_client_curl.cc @@ -77,9 +77,12 @@ void Session::SendRequest( // reuse it instead of creating a new one. http_client_.MaybeSpawnBackgroundThread(); } - else if (callback) + else { - callback->OnEvent(opentelemetry::ext::http::client::SessionState::CreateFailed, ""); + if (callback) + { + callback->OnEvent(opentelemetry::ext::http::client::SessionState::CreateFailed, ""); + } is_session_active_.store(false, std::memory_order_release); } } @@ -176,8 +179,9 @@ bool HttpClient::CancelAllSessions() noexcept { std::unordered_map> sessions; { + // We can only cleanup session and curl handles in the IO thread. std::lock_guard lock_guard{sessions_m_}; - sessions.swap(sessions_); + sessions = sessions_; } if (sessions.empty()) @@ -200,8 +204,9 @@ bool HttpClient::FinishAllSessions() noexcept { std::unordered_map> sessions; { + // We can only cleanup session and curl handles in the IO thread. std::lock_guard lock_guard{sessions_m_}; - sessions.swap(sessions_); + sessions = sessions_; } if (sessions.empty()) diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index a87cf5083e..b46b73fdd7 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -1,6 +1,8 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include + #include "opentelemetry/ext/http/client/curl/http_operation_curl.h" #include "opentelemetry/ext/http/client/curl/http_client_curl.h"