From e35bb01f6e0a99d6fba21dbe106eace4718e697e Mon Sep 17 00:00:00 2001 From: Mike Nugent Date: Wed, 5 Aug 2026 18:29:39 +0000 Subject: [PATCH 1/6] [BUG] Prevent lost condition-variable wakeups during OTLP file, periodic metric, and batch span processor shutdown. --- exporters/otlp/src/otlp_file_client.cc | 16 +++++++++++----- .../export/periodic_exporting_metric_reader.cc | 5 +++++ sdk/src/trace/batch_span_processor.cc | 10 +++++++++- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc index 5bcbedfbe5..1398135b72 100644 --- a/exporters/otlp/src/otlp_file_client.cc +++ b/exporters/otlp/src/otlp_file_client.cc @@ -994,6 +994,10 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender { if (file_) { + { + std::lock_guard waker_guard{file_->background_thread_waker_lock}; + file_->is_shutdown.store(true, std::memory_order_release); + } file_->background_thread_waker_cv.notify_all(); std::unique_ptr background_flush_thread; { @@ -1482,11 +1486,6 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender break; } - if (concurrency_file->is_shutdown.load(std::memory_order_acquire)) - { - break; - } - #ifdef ENABLE_THREAD_INSTRUMENTATION_PREVIEW if (thread_instrumentation != nullptr) { @@ -1496,6 +1495,13 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender { std::unique_lock lk(concurrency_file->background_thread_waker_lock); + // Even though is_shutdown is atomic, the lock guarantees that either a change to + // is_shutdown will be observed, or background_thread_waker_cv will see the notification + // at shutdown. + if (concurrency_file->is_shutdown.load(std::memory_order_acquire)) + { + break; + } concurrency_file->background_thread_waker_cv.wait_for(lk, flush_interval); } diff --git a/sdk/src/metrics/export/periodic_exporting_metric_reader.cc b/sdk/src/metrics/export/periodic_exporting_metric_reader.cc index 86206ad84e..cbf95d5441 100644 --- a/sdk/src/metrics/export/periodic_exporting_metric_reader.cc +++ b/sdk/src/metrics/export/periodic_exporting_metric_reader.cc @@ -283,6 +283,11 @@ bool PeriodicExportingMetricReader::OnShutDown(std::chrono::microseconds timeout { if (worker_thread_.joinable()) { + { + // Acquiring cv_m_ guarantees that the next time the worker thread checks the wait condition + // on cv_ (either from notify below or any other reason) it will see IsShutdown() return true. + std::lock_guard cv_guard{cv_m_}; + } cv_.notify_all(); worker_thread_.join(); } diff --git a/sdk/src/trace/batch_span_processor.cc b/sdk/src/trace/batch_span_processor.cc index c59c9866a2..6e4744b674 100644 --- a/sdk/src/trace/batch_span_processor.cc +++ b/sdk/src/trace/batch_span_processor.cc @@ -376,7 +376,15 @@ bool BatchSpanProcessor::InternalShutdown(std::chrono::microseconds timeout) noe if (worker_thread_.joinable()) { - synchronization_data_->is_force_wakeup_background_worker.store(true, std::memory_order_release); + { + // Even though is_force_wakeup_background_worker is atomic, acquiring cv_m is necessary as + // otherwise both the change to is_force_wakeup_background_worker and the notify_all might + // occur after the worker has checked is_force_wakeup_background_worker but before it has + // blocked again on synchronization_data_->cv. + std::lock_guard cv_guard{synchronization_data_->cv_m}; + synchronization_data_->is_force_wakeup_background_worker.store(true, + std::memory_order_release); + } synchronization_data_->cv.notify_all(); worker_thread_.join(); } From 6551f6028b50caa3531e30c2c211f17d36faa44f Mon Sep 17 00:00:00 2001 From: Mike Nugent Date: Wed, 19 Aug 2026 16:29:18 +0000 Subject: [PATCH 2/6] Revert changes to batch_span_processor.cc --- sdk/src/trace/batch_span_processor.cc | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/sdk/src/trace/batch_span_processor.cc b/sdk/src/trace/batch_span_processor.cc index 6e4744b674..c59c9866a2 100644 --- a/sdk/src/trace/batch_span_processor.cc +++ b/sdk/src/trace/batch_span_processor.cc @@ -376,15 +376,7 @@ bool BatchSpanProcessor::InternalShutdown(std::chrono::microseconds timeout) noe if (worker_thread_.joinable()) { - { - // Even though is_force_wakeup_background_worker is atomic, acquiring cv_m is necessary as - // otherwise both the change to is_force_wakeup_background_worker and the notify_all might - // occur after the worker has checked is_force_wakeup_background_worker but before it has - // blocked again on synchronization_data_->cv. - std::lock_guard cv_guard{synchronization_data_->cv_m}; - synchronization_data_->is_force_wakeup_background_worker.store(true, - std::memory_order_release); - } + synchronization_data_->is_force_wakeup_background_worker.store(true, std::memory_order_release); synchronization_data_->cv.notify_all(); worker_thread_.join(); } From b28f264f041adf3ff4e57d378096711de4c5616e Mon Sep 17 00:00:00 2001 From: Mike Nugent Date: Wed, 19 Aug 2026 18:41:32 +0000 Subject: [PATCH 3/6] Update changelog and fix additional races --- CHANGELOG.md | 3 +++ exporters/otlp/src/otlp_file_client.cc | 5 ++++- sdk/src/metrics/export/periodic_exporting_metric_reader.cc | 7 ++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f584c3b765..f8295b0838 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ Increment the: ## [Unreleased] +* [BUG] Prevent lost condition-variable wakeups in OTLP file exporter and periodic + metric exporter. + * [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 diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc index 1398135b72..44a6e570ba 100644 --- a/exporters/otlp/src/otlp_file_client.cc +++ b/exporters/otlp/src/otlp_file_client.cc @@ -1137,7 +1137,10 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender bool Shutdown(std::chrono::microseconds timeout) noexcept override { - file_->is_shutdown.store(true, std::memory_order_release); + { + std::lock_guard waker_guard{file_->background_thread_waker_lock}; + file_->is_shutdown.store(true, std::memory_order_release); + } bool result = ForceFlush(timeout); return result; diff --git a/sdk/src/metrics/export/periodic_exporting_metric_reader.cc b/sdk/src/metrics/export/periodic_exporting_metric_reader.cc index cbf95d5441..32b7d3d0d0 100644 --- a/sdk/src/metrics/export/periodic_exporting_metric_reader.cc +++ b/sdk/src/metrics/export/periodic_exporting_metric_reader.cc @@ -221,7 +221,12 @@ bool PeriodicExportingMetricReader::OnForceFlush(std::chrono::microseconds timeo if (force_flush_pending_sequence_.load(std::memory_order_acquire) > force_flush_notified_sequence_.load(std::memory_order_acquire)) { - is_force_wakeup_background_worker_.store(true, std::memory_order_release); + { + // Acquiring cv_m_ guarantees that the worker thread either is not currently waiting on cv_, + // or the notify below will cause it to re-check the wait condition. + std::lock_guard cv_guard{cv_m_}; + is_force_wakeup_background_worker_.store(true, std::memory_order_release); + } cv_.notify_all(); } return force_flush_notified_sequence_.load(std::memory_order_acquire) >= current_sequence; From c2e05e441235f72fb35c852dbe7f855356b6cbb4 Mon Sep 17 00:00:00 2001 From: Mike Nugent Date: Wed, 19 Aug 2026 19:23:29 +0000 Subject: [PATCH 4/6] Add PR to changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 490a601bcc..bac36728ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,8 @@ Increment the: ## [Unreleased] * [BUG] Prevent lost condition-variable wakeups in OTLP file exporter and periodic - metric exporter. + metric exporter + [#4365](https://github.com/open-telemetry/opentelemetry-cpp/pull/4365) * [CONFIGURATION] Build the configured resource detectors in SdkBuilder, apply the `detection.attributes` include/exclude filter to the detected attributes, From 6b7a1dbcfadef48711b07de55cf55234312a2fd4 Mon Sep 17 00:00:00 2001 From: Mike Nugent Date: Thu, 20 Aug 2026 13:02:06 +0000 Subject: [PATCH 5/6] Break after AfterWait --- exporters/otlp/src/otlp_file_client.cc | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc index 44a6e570ba..fd4f73f47c 100644 --- a/exporters/otlp/src/otlp_file_client.cc +++ b/exporters/otlp/src/otlp_file_client.cc @@ -1496,16 +1496,17 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender } #endif /* ENABLE_THREAD_INSTRUMENTATION_PREVIEW */ + bool is_shutdown = false; { std::unique_lock lk(concurrency_file->background_thread_waker_lock); // Even though is_shutdown is atomic, the lock guarantees that either a change to // is_shutdown will be observed, or background_thread_waker_cv will see the notification // at shutdown. - if (concurrency_file->is_shutdown.load(std::memory_order_acquire)) + is_shutdown = concurrency_file->is_shutdown.load(std::memory_order_acquire); + if (!is_shutdown) { - break; + concurrency_file->background_thread_waker_cv.wait_for(lk, flush_interval); } - concurrency_file->background_thread_waker_cv.wait_for(lk, flush_interval); } #ifdef ENABLE_THREAD_INSTRUMENTATION_PREVIEW @@ -1515,6 +1516,11 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender } #endif /* ENABLE_THREAD_INSTRUMENTATION_PREVIEW */ + if (is_shutdown) + { + break; + } + { std::size_t current_record_count = concurrency_file->record_count.load(std::memory_order_acquire); From f39c117f156d2648f26bbece47cd111f336d0ecd Mon Sep 17 00:00:00 2001 From: Mike Nugent Date: Mon, 24 Aug 2026 13:30:55 +0000 Subject: [PATCH 6/6] Update comment around is_shutdown --- exporters/otlp/src/otlp_file_client.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc index fd4f73f47c..d881352ae8 100644 --- a/exporters/otlp/src/otlp_file_client.cc +++ b/exporters/otlp/src/otlp_file_client.cc @@ -1501,7 +1501,9 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender std::unique_lock lk(concurrency_file->background_thread_waker_lock); // Even though is_shutdown is atomic, the lock guarantees that either a change to // is_shutdown will be observed, or background_thread_waker_cv will see the notification - // at shutdown. + // at shutdown. It is important to set is_shutdown prior to `wait_for` rather than + // as part of a condition in `wait_for` so that a shutdown while the thread is in + // `wait_for` will still call `std::fflush` below. is_shutdown = concurrency_file->is_shutdown.load(std::memory_order_acquire); if (!is_shutdown) {