Skip to content

Commit

Permalink
Fix infinitely waiting when shutdown with more than one running http …
Browse files Browse the repository at this point in the history
…sessions. (#1549)

Signed-off-by: owentou <owentou@tencent.com>
  • Loading branch information
owent authored Aug 10, 2022
1 parent 319d854 commit ba1cd0d
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
16 changes: 12 additions & 4 deletions exporters/otlp/src/otlp_http_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 9 additions & 4 deletions ext/src/http/client/curl/http_client_curl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -176,8 +179,9 @@ bool HttpClient::CancelAllSessions() noexcept
{
std::unordered_map<uint64_t, std::shared_ptr<Session>> sessions;
{
// We can only cleanup session and curl handles in the IO thread.
std::lock_guard<std::mutex> lock_guard{sessions_m_};
sessions.swap(sessions_);
sessions = sessions_;
}

if (sessions.empty())
Expand All @@ -200,8 +204,9 @@ bool HttpClient::FinishAllSessions() noexcept
{
std::unordered_map<uint64_t, std::shared_ptr<Session>> sessions;
{
// We can only cleanup session and curl handles in the IO thread.
std::lock_guard<std::mutex> lock_guard{sessions_m_};
sessions.swap(sessions_);
sessions = sessions_;
}

if (sessions.empty())
Expand Down
2 changes: 2 additions & 0 deletions ext/src/http/client/curl/http_operation_curl.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#include <cstring>

#include "opentelemetry/ext/http/client/curl/http_operation_curl.h"

#include "opentelemetry/ext/http/client/curl/http_client_curl.h"
Expand Down

0 comments on commit ba1cd0d

Please sign in to comment.