From f02d70bdb82985bea713b283bdd3da9937cb2172 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 3 Sep 2024 09:19:57 -0400 Subject: [PATCH] Fix overflow in timeout logic (#3046) --- .../periodic_exporting_metric_reader.cc | 2 +- sdk/src/metrics/meter_context.cc | 34 +++++++------------ 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/sdk/src/metrics/export/periodic_exporting_metric_reader.cc b/sdk/src/metrics/export/periodic_exporting_metric_reader.cc index 1ebfc2a1b6..42fd6003ae 100644 --- a/sdk/src/metrics/export/periodic_exporting_metric_reader.cc +++ b/sdk/src/metrics/export/periodic_exporting_metric_reader.cc @@ -218,7 +218,7 @@ bool PeriodicExportingMetricReader::OnForceFlush(std::chrono::microseconds timeo // - If original `timeout` is `zero`, use that in exporter::forceflush // - Else if remaining `timeout_steady` more than zero, use that in exporter::forceflush // - Else don't invoke exporter::forceflush ( as remaining time is zero or less) - if (timeout <= std::chrono::steady_clock::duration::zero()) + if (timeout <= std::chrono::milliseconds::duration::zero()) { result = exporter_->ForceFlush(std::chrono::duration_cast(timeout)); diff --git a/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc index 40b9da42b2..42efce76fb 100644 --- a/sdk/src/metrics/meter_context.cc +++ b/sdk/src/metrics/meter_context.cc @@ -168,46 +168,36 @@ bool MeterContext::ForceFlush(std::chrono::microseconds timeout) noexcept bool result = true; // Simultaneous flush not allowed. const std::lock_guard locked(forceflush_lock_); - // Convert to nanos to prevent overflow - auto timeout_ns = (std::chrono::nanoseconds::max)(); - if (std::chrono::duration_cast(timeout_ns) > timeout) - { - timeout_ns = std::chrono::duration_cast(timeout); - } - - auto current_time = std::chrono::system_clock::now(); - std::chrono::system_clock::time_point expire_time; - auto overflow_checker = (std::chrono::system_clock::time_point::max)(); - // check if the expected expire time doesn't overflow. - if (overflow_checker - current_time > timeout_ns) + auto time_remaining = (std::chrono::steady_clock::duration::max)(); + if (std::chrono::duration_cast(time_remaining) > timeout) { - expire_time = - current_time + std::chrono::duration_cast(timeout_ns); + time_remaining = timeout; } - else + + auto current_time = std::chrono::steady_clock::now(); + auto expire_time = (std::chrono::steady_clock::time_point::max)(); + if (expire_time - current_time > time_remaining) { - // overflow happens, reset expire time to max. - expire_time = overflow_checker; + expire_time = current_time + time_remaining; } for (auto &collector : collectors_) { if (!std::static_pointer_cast(collector)->ForceFlush( - std::chrono::duration_cast(timeout_ns))) + std::chrono::duration_cast(time_remaining))) { result = false; } - current_time = std::chrono::system_clock::now(); - + current_time = std::chrono::steady_clock::now(); if (expire_time >= current_time) { - timeout_ns = std::chrono::duration_cast(expire_time - current_time); + time_remaining = expire_time - current_time; } else { - timeout_ns = std::chrono::nanoseconds::zero(); + time_remaining = std::chrono::steady_clock::duration::zero(); } } if (!result)