From 94a1168a5832a764107f05835f54ebf922069b72 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Sun, 1 Sep 2024 15:13:56 -0400 Subject: [PATCH] Fix overflow in timeout logic Also use steady clock consistently. Prior to this change, the test would fail under ASAN: bazel test --config=asan --test_output=errors //sdk/test/metrics:meter_provider_sdk_test INFO: Analyzed target //sdk/test/metrics:meter_provider_sdk_test (0 packages loaded, 0 targets configured). FAIL: //sdk/test/metrics:meter_provider_sdk_test (see /private/var/tmp/_bazel_punya/e3bd968ba61238cdeb1a5537fc3dbf7d/execroot/_main/bazel-out/darwin_arm64-fastbuild-asan/testlogs/sdk/test/metrics/meter_provider_sdk_test/test.log) INFO: From Testing //sdk/test/metrics:meter_provider_sdk_test: ==================== Test output for //sdk/test/metrics:meter_provider_sdk_test: Running main() from gmock_main.cc [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from MeterProvider [ RUN ] MeterProvider.GetMeter [Warning] File: sdk/src/metrics/meter_provider.cc:65 [MeterProvider::GetMeter] Library name is empty. [Warning] File: sdk/src/metrics/meter_provider.cc:65 [MeterProvider::GetMeter] Library name is empty. /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__chrono/duration.h:102:59: runtime error: signed integer overflow: 9221646818050376183 * 1000 cannot be represented in type '_Ct' (aka 'long long') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__chrono/duration.h:102:59 in ================================================================================ INFO: Found 1 test target... Target //sdk/test/metrics:meter_provider_sdk_test up-to-date: bazel-bin/sdk/test/metrics/meter_provider_sdk_test INFO: Elapsed time: 2.251s, Critical Path: 2.13s INFO: 5 processes: 5 darwin-sandbox. INFO: Build completed, 1 test FAILED, 5 total actions //sdk/test/metrics:meter_provider_sdk_test FAILED in 0.6s /private/var/tmp/_bazel_punya/e3bd968ba61238cdeb1a5537fc3dbf7d/execroot/_main/bazel-out/darwin_arm64-fastbuild-asan/testlogs/sdk/test/metrics/meter_provider_sdk_test/test.log Executed 1 out of 1 test: 1 fails locally. --- sdk/src/metrics/meter_context.cc | 34 +++++++++++--------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc index 40b9da42b2..2007f69441 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)