Skip to content

Commit 8d5282a

Browse files
Merge branch 'main' into fix/4625-narrow-ext-headers
2 parents 3a62f7b + 3f1a8cf commit 8d5282a

7 files changed

Lines changed: 81 additions & 17 deletions

File tree

‎.bazelversion‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
8.8.0
1+
8.8.1

‎.github/workflows/codeql-analysis.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ jobs:
5757
run: |
5858
sudo -E ./ci/install_thirdparty.sh --install-dir /usr/local --tags-file third_party_release --packages "ryml"
5959
- name: Initialize CodeQL
60-
uses: github/codeql-action/init@1c5b675653bb5c22dbe9b12b556ec555138e09fd # v4.38.1
60+
uses: github/codeql-action/init@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2 # v4.38.2
6161
with:
6262
languages: cpp
6363
config: |
@@ -72,4 +72,4 @@ jobs:
7272
"${GITHUB_WORKSPACE}"
7373
cmake --build . --parallel
7474
- name: Perform CodeQL Analysis
75-
uses: github/codeql-action/analyze@1c5b675653bb5c22dbe9b12b556ec555138e09fd # v4.38.1
75+
uses: github/codeql-action/analyze@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2 # v4.38.2

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,12 @@ Increment the:
9292

9393
* [BUG] Do not queue a curl session closed by the Retry-After cap
9494
[#4632](https://github.com/open-telemetry/opentelemetry-cpp/pull/4632)
95+
* [BUG] Fix a data race on the curl operation's last result code
96+
[#4618](https://github.com/open-telemetry/opentelemetry-cpp/pull/4618)
97+
98+
* [BUG] Do not busy-spin the curl background thread while a retry is queued
99+
at shutdown
100+
[#4637](https://github.com/open-telemetry/opentelemetry-cpp/pull/4637)
95101

96102
## [1.29.0] 2026-09-13
97103

‎api/include/opentelemetry/common/spin_lock_mutex.h‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,7 @@ constexpr int SPINLOCK_SLEEP_MS = 1;
3535
* 2. A loop where the current thread yields control after checking the lock.
3636
* 3. Issuing a thread-sleep call before starting back in phase 1.
3737
*
38-
* This is meant to give a good balance of perofrmance and CPU consumption in
39-
* practice.
40-
*
41-
* This mutex uses an incremental back-off strategy with the following phases:
42-
* 1. A tight spin-lock loop (pending: using hardware PAUSE/YIELD instructions)
43-
* 2. A loop where the current thread yields control after checking the lock.
44-
* 3. Issuing a thread-sleep call before starting back in phase 1.
45-
*
46-
* This is meant to give a good balance of perofrmance and CPU consumption in
38+
* This is meant to give a good balance of performance and CPU consumption in
4739
* practice.
4840
*
4941
* This class implements the `BasicLockable` specification:
@@ -119,7 +111,7 @@ class SpinLockMutex
119111
{
120112
return;
121113
}
122-
// Sleep and then start the whole process again. (goal ~1000ns)
114+
// Sleep and then start the whole process again. (goal ~1ms)
123115
std::this_thread::sleep_for(std::chrono::milliseconds(SPINLOCK_SLEEP_MS));
124116
}
125117
return;

‎ext/src/http/client/curl/http_client_curl.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,8 @@ bool HttpClient::MaybeSpawnBackgroundThread()
635635
if (self->doRetrySessions(true))
636636
{
637637
still_running = 1;
638+
// With wait_for zero, as during shutdown, poll here until a queued retry is due.
639+
need_wait_more = true;
638640
}
639641

640642
// If there is no pending jobs, we can stop the background thread.

‎ext/src/http/client/curl/http_operation_curl.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -508,8 +508,8 @@ HttpOperation::~HttpOperation()
508508
{
509509
if (HttpOperationAccessor::GetThreadId(*async_data_) != std::this_thread::get_id())
510510
{
511-
async_data_->result_future.wait();
512-
last_curl_result_ = async_data_->result_future.get();
511+
// PerformCurlMessage() stores this, then calls the Cleanup() that sets the promise.
512+
static_cast<void>(async_data_->result_future.get());
513513
}
514514
}
515515
break;
@@ -533,8 +533,8 @@ void HttpOperation::Finish()
533533
// We should not wait in callback from Cleanup()
534534
if (HttpOperationAccessor::GetThreadId(*async_data_) != std::this_thread::get_id())
535535
{
536-
async_data_->result_future.wait();
537-
last_curl_result_ = async_data_->result_future.get();
536+
// PerformCurlMessage() stores this, then calls the Cleanup() that sets the promise.
537+
static_cast<void>(async_data_->result_future.get());
538538
}
539539
}
540540
}

‎ext/test/http/curl_http_test.cc‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,13 @@
66
#include "gtest/gtest.h"
77

88
#ifdef ENABLE_OTLP_RETRY_PREVIEW
9+
# include <ratio>
910
# include "gmock/gmock.h"
11+
# ifdef _WIN32
12+
# include <windows.h>
13+
# else
14+
# include <ctime>
15+
# endif
1016
#endif // ENABLE_OTLP_RETRY_PREVIEW
1117

1218
#ifdef ENABLE_OTLP_COMPRESSION_PREVIEW
@@ -770,6 +776,64 @@ TEST_F(BasicCurlHttpTests, RetryAfterBeyondMaxBackoffDoesNotDelayShutdown)
770776
return received.uri == "/retry-after/";
771777
}));
772778
}
779+
780+
// CPU time of the whole process. std::clock() measures wall time on Windows.
781+
std::chrono::microseconds ProcessCpuTime()
782+
{
783+
# ifdef _WIN32
784+
FILETIME created{}, exited{}, kernel{}, user{};
785+
GetProcessTimes(GetCurrentProcess(), &created, &exited, &kernel, &user);
786+
// FILETIME counts 100 ns ticks.
787+
const auto ticks = [](const FILETIME &time) {
788+
return (static_cast<std::uint64_t>(time.dwHighDateTime) << 32) | time.dwLowDateTime;
789+
};
790+
return std::chrono::microseconds{static_cast<std::int64_t>((ticks(kernel) + ticks(user)) / 10)};
791+
# else
792+
return std::chrono::microseconds{
793+
static_cast<std::int64_t>(static_cast<double>(std::clock()) * 1000000 / CLOCKS_PER_SEC)};
794+
# endif
795+
}
796+
797+
// During shutdown the IO loop used to skip its poll while a retry waited out its backoff.
798+
TEST_F(BasicCurlHttpTests, ShutdownDoesNotSpinWhileARetryIsQueued)
799+
{
800+
received_requests_.clear();
801+
curl::HttpClient http_client;
802+
// Three waits: the first can still sleep on a poll the loop asked for before the join.
803+
const http_client::RetryPolicy retry_policy = {4, std::chrono::duration<float>{0.25f},
804+
std::chrono::duration<float>{5.0f}, 2.0f};
805+
806+
auto session = http_client.CreateSession("http://127.0.0.1:19000");
807+
auto request = session->CreateRequest();
808+
request->SetMethod(http_client::Method::Post);
809+
request->SetUri("retry/");
810+
request->SetRetryPolicy(retry_policy);
811+
auto handler = std::make_shared<RetryEventHandler>();
812+
session->SendRequest(handler);
813+
ASSERT_TRUE(waitForRequests(30, 1));
814+
815+
const auto cpu_before = ProcessCpuTime();
816+
const auto started_at = std::chrono::steady_clock::now();
817+
http_client.WaitBackgroundThreadExit();
818+
const auto joined_in = std::chrono::steady_clock::now() - started_at;
819+
const auto cpu_used = ProcessCpuTime() - cpu_before;
820+
821+
session->FinishSession();
822+
ASSERT_TRUE(handler->got_response_.load(std::memory_order_acquire));
823+
824+
std::unique_lock<std::mutex> lock_requests(mtx_requests);
825+
EXPECT_EQ(4, std::count_if(received_requests_.begin(), received_requests_.end(),
826+
[](const HTTP_SERVER_NS::HttpRequest &received) {
827+
return received.uri == "/retry/";
828+
}));
829+
830+
# ifdef __APPLE__
831+
GTEST_SKIP() << "The test server's kqueue reactor spins while a connection is open";
832+
# endif
833+
EXPECT_TRUE(cpu_used * 2 < joined_in)
834+
<< "join ms: " << std::chrono::duration_cast<std::chrono::milliseconds>(joined_in).count()
835+
<< ", CPU ms: " << std::chrono::duration_cast<std::chrono::milliseconds>(cpu_used).count();
836+
}
773837
#endif // ENABLE_OTLP_RETRY_PREVIEW
774838

775839
// A cancel that arrives once the server has answered used to deliver Cancelled and the response,

0 commit comments

Comments
 (0)