From a0dae9bd31e7802a788be95b0acde0b39515c773 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:14:11 +0000 Subject: [PATCH 1/2] [BUG] Draw the retry jitter from a per thread generator NextRetryTime multiplied the backoff by a sample from a function local static std::mt19937. Drawing from an engine advances its state, and every HttpClient runs its own background thread, so two clients retrying at the same time wrote the same engine. Thread safe initialisation of a function local static says nothing about using one. The engine is thread_local now and the distribution is an ordinary local, since sharing it bought nothing either. Each thread seeds its own from a temporary random_device, so the jitter is still per draw. Fixes #4398. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- .../http/client/curl/http_operation_curl.cc | 7 +++-- ext/test/http/curl_http_test.cc | 31 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index d6f49e061c..00b451de85 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -618,9 +618,10 @@ std::chrono::system_clock::time_point HttpOperation::NextRetryTime() return retry_after_time_point_; } - static std::random_device rd; - static std::mt19937 gen(rd()); - static std::uniform_real_distribution dis(0.8f, 1.2f); + // One engine per thread. Every HttpClient drives its own background thread, and drawing from + // the engine advances its state, so a shared one is written by all of them at once. + static thread_local std::mt19937 gen{std::random_device{}()}; + std::uniform_real_distribution dis(0.8f, 1.2f); // The initial retry attempt will occur after initialBackoff * random(0.8, 1.2) auto backoff = retry_policy_.initial_backoff; diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index bfd9153743..e1b180e8fe 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -562,6 +562,37 @@ TEST_F(BasicCurlHttpTests, ACancelBeforeTheResponseReportsCancelled) session_manager->FinishAllSessions(); } +// NextRetryTime draws the backoff jitter from an engine that used to be a shared static, so +// two clients retrying at the same time wrote the same std::mt19937. The case passes either +// way, since a data race is not a functional failure. It is here for the sanitizer builds. +TEST_F(BasicCurlHttpTests, RetryJitterIsNotSharedAcrossThreads) +{ + opentelemetry::ext::http::client::HttpSslOptions ssl_options; + opentelemetry::ext::http::client::Headers request_headers; + opentelemetry::ext::http::client::Body request_body; + + http_client::curl::HttpOperation first(http_client::Method::Get, "http://127.0.0.1:19000/", + ssl_options, nullptr, request_headers, request_body); + http_client::curl::HttpOperation second(http_client::Method::Get, "http://127.0.0.1:19000/", + ssl_options, nullptr, request_headers, request_body); + + std::thread drawing_first([&first] { + for (int i = 0; i < 200; ++i) + { + (void)first.NextRetryTime(); + } + }); + std::thread drawing_second([&second] { + for (int i = 0; i < 200; ++i) + { + (void)second.NextRetryTime(); + } + }); + + drawing_first.join(); + drawing_second.join(); +} + TEST_F(BasicCurlHttpTests, SendGetRequestSync) { received_requests_.clear(); From ee6ded9a2e4b41a03586691db1d037328e8ed684 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:14:59 +0000 Subject: [PATCH 2/2] [BUG] Draw the retry jitter from a per thread generator Add the CHANGELOG entry now that the pull request has a number. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f1d54a64a..5bca7b4b1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,9 @@ Increment the: * [BUG] Report one outcome per request when a curl session is cancelled after the response arrives ([#4363](https://github.com/open-telemetry/opentelemetry-cpp/pull/4363)) +* [BUG] Draw the curl retry jitter from a per thread generator instead of one + shared across HTTP client threads + ([#4399](https://github.com/open-telemetry/opentelemetry-cpp/pull/4399)) * [CODE HEALTH] Enable clang-tidy `modernize-deprecated-headers` and replace deprecated C headers (`stdint.h`, `stddef.h`, `stdlib.h`, `string.h`, `stdio.h`, `ctype.h`, `limits.h`, `assert.h`) with their C++ equivalents