From 410061b15a9b0a464c851173fa568e49c85570dc Mon Sep 17 00:00:00 2001
From: Alexandre Oliva <oliva@adacore.com>
Date: Mon, 2 Sep 2024 11:31:59 -0300
Subject: [PATCH] [libstdc++] [testsuite] avoid async.cc loss of precision
[PR91486]
When we get to test_pr91486_wait_until(), we're about 10s past the
float_steady_clock epoch. This is enough for the 1s delta for the
timeout to come out slightly lower when the futex-less wait_until
converts the deadline from float_steady_clock to __clock_t. So we may
wake up a little too early, and end up looping one extra time to sleep
for e.g. another 954ns until we hit the deadline.
Each iteration calls float_steady_clock::now(), bumping the call_count
that we VERIFY() at the end of the subtest. Since we expect at most 3
calls, and we're going to have at the very least 3 on futex-less
targets (one in the test proper, one before wait_until_impl to compute
the deadline, and one after wait_until_impl to check whether the
deadline was hit), any such imprecision that causes an extra iteration
will reach 5 and cause the test to fail.
Initializing the epoch in the beginning of the test makes such
spurious fails due to loss of precision far less likely. I don't
suppose allowing for an extra couple of calls would be desirable.
While at that, I'm annotating unused status variables as such.
for libstdc++-v3/ChangeLog
PR libstdc++/91486
* testsuite/30_threads/async/async.cc
(test_pr91486_wait_for): Mark status as unused.
(test_pr91486_wait_until): Likewise. Initialize epoch later.
---
.../testsuite/30_threads/async/async.cc | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/libstdc++-v3/testsuite/30_threads/async/async.cc b/libstdc++-v3/testsuite/30_threads/async/async.cc
index 3b157ed9c568..2474d318d7b1 100644
--- a/libstdc++-v3/testsuite/30_threads/async/async.cc
+++ b/libstdc++-v3/testsuite/30_threads/async/async.cc
@@ -173,7 +173,7 @@ void test_pr91486_wait_for()
std::chrono::duration<float> const wait_time = std::chrono::seconds(1);
auto const start_steady = chrono::steady_clock::now();
- auto status = f1.wait_for(wait_time);
+ auto status __attribute__ ((__unused__)) = f1.wait_for(wait_time);
auto const elapsed_steady = chrono::steady_clock::now() - start_steady;
VERIFY( elapsed_steady >= std::chrono::seconds(1) );
@@ -209,7 +209,7 @@ struct float_steady_clock
}
};
-chrono::steady_clock::time_point float_steady_clock::epoch = chrono::steady_clock::now();
+chrono::steady_clock::time_point float_steady_clock::epoch;
int float_steady_clock::call_count = 0;
void test_pr91486_wait_until()
@@ -218,6 +218,19 @@ void test_pr91486_wait_until()
std::this_thread::sleep_for(std::chrono::seconds(1));
});
+ // When we don't _GLIBCXX_HAVE_LINUX_FUTEX, we use
+ // condition_variables, whose wait_until converts times using
+ // deltas, and if too much time has elapsed since we set the epoch
+ // during program initialization, say if the other tests took over
+ // 8s and we're unlucky with the numbers, we may lose enough
+ // precision from the 1s delta that we don't sleep until the
+ // deadline, and then we may loop more times than expected. Each
+ // iteration will recompute the wait time from deadline -
+ // float_steady_clock::now(), and each such computation will bump
+ // float_steady_clock::call_count, so the call_count check below
+ // will fail spuriously. Setting the epoch just before running this
+ // test makes this failure mode far less likely.
+ float_steady_clock::epoch = chrono::steady_clock::now();
float_steady_clock::time_point const now = float_steady_clock::now();
std::chrono::duration<float> const wait_time = std::chrono::seconds(1);
@@ -225,7 +238,7 @@ void test_pr91486_wait_until()
VERIFY( expire > now );
auto const start_steady = chrono::steady_clock::now();
- auto status = f1.wait_until(expire);
+ auto status __attribute__ ((__unused__)) = f1.wait_until(expire);
auto const elapsed_steady = chrono::steady_clock::now() - start_steady;
// This checks that we didn't come back too soon
--
GitLab