Skip to content

Commit

Permalink
Fix overflow in timeout logic
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
punya committed Sep 1, 2024
1 parent a920898 commit 94a1168
Showing 1 changed file with 12 additions and 22 deletions.
34 changes: 12 additions & 22 deletions sdk/src/metrics/meter_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,46 +168,36 @@ bool MeterContext::ForceFlush(std::chrono::microseconds timeout) noexcept
bool result = true;
// Simultaneous flush not allowed.
const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(forceflush_lock_);
// Convert to nanos to prevent overflow
auto timeout_ns = (std::chrono::nanoseconds::max)();
if (std::chrono::duration_cast<std::chrono::microseconds>(timeout_ns) > timeout)
{
timeout_ns = std::chrono::duration_cast<std::chrono::nanoseconds>(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<std::chrono::microseconds>(time_remaining) > timeout)
{
expire_time =
current_time + std::chrono::duration_cast<std::chrono::system_clock::duration>(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<MetricCollector>(collector)->ForceFlush(
std::chrono::duration_cast<std::chrono::microseconds>(timeout_ns)))
std::chrono::duration_cast<std::chrono::microseconds>(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<std::chrono::nanoseconds>(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)
Expand Down

0 comments on commit 94a1168

Please sign in to comment.