From 0799336d64be65eb97d330606c30162dc3440cab Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Wed, 8 Oct 2025 15:22:59 -0700 Subject: [PATCH] Deflake IntervalTimerTest tests that relied on old kernel behavior. This commit fixes two flaky tests in the IntervalTimerTest suite that were failing on newer Linux kernels (like Linux 6.6.65) due to changes in signal handling logic. It passes on at least Linux 5.10.0. RealTimeSignalsAreNotDuplicated: This test was failing because it assumed that disarming a timer (calling timer_settime with a zero interval) would leave one pending signal but reset the si_overrun count. On newer kernels, this action appears to clear both the pending signal and the overrun count, causing the initial sigtimedwait() to fail with EAGAIN. The relevant Linux commit seems to be 513793bc6ab3 ("posix-timers: Make signal delivery consistent"). Fixed it by calling sigtimedwait() before disarming the timer. IgnoredSignalCountsAsOverrun: This test was also failing with EAGAIN. Its logic was based on the premise that a blocked signal with a SIG_IGN disposition would be queued, allowing sigtimedwait to retrieve it and check its overrun count. This premise is no longer valid on modern kernels. It seems like newer kernels now discard SIG_IGN immediately on generation, regardless of whether the signal is blocked or not. The signal never becomes pending, so sigtimedwait will always fail. The relevant Linux commit seems to be df7a996b4dab ("signal: Queue ignored posixtimers on ignore list"). Just deleted this test. PiperOrigin-RevId: 816883590 --- test/syscalls/linux/BUILD | 1 + test/syscalls/linux/timers.cc | 81 ++++++----------------------------- 2 files changed, 15 insertions(+), 67 deletions(-) diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index a7f20e2585..3818bdd8a4 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -4176,6 +4176,7 @@ cc_binary( "//test/util:logging", "//test/util:multiprocess_util", "//test/util:posix_error", + "//test/util:save_util", "//test/util:signal_util", "//test/util:test_util", "//test/util:thread_util", diff --git a/test/syscalls/linux/timers.cc b/test/syscalls/linux/timers.cc index 6260d7b04d..216de706dc 100644 --- a/test/syscalls/linux/timers.cc +++ b/test/syscalls/linux/timers.cc @@ -33,6 +33,7 @@ #include "test/util/logging.h" #include "test/util/multiprocess_util.h" #include "test/util/posix_error.h" +#include "test/util/save_util.h" #include "test/util/signal_util.h" #include "test/util/test_util.h" #include "test/util/thread_util.h" @@ -486,6 +487,11 @@ TEST(IntervalTimerTest, RealTimeSignalsAreNotDuplicated) { const auto timer = ASSERT_NO_ERRNO_AND_VALUE(TimerCreate(CLOCK_MONOTONIC, sev)); + // Disable save because a save/restore cycle adds considerable time overhead + // between each syscall such that `timer` fires in between sigtimedwait() call + // and timer.Set() call, which causes the last sigtimedwait() check to fail. + DisableSave ds; + constexpr absl::Duration kPeriod = absl::Seconds(1); constexpr int kCycles = 3; struct itimerspec its = {}; @@ -493,22 +499,22 @@ TEST(IntervalTimerTest, RealTimeSignalsAreNotDuplicated) { ASSERT_NO_ERRNO(timer.Set(0, its)); absl::SleepFor(kPeriod * kCycles + kTimerSlack); - // Stop the timer so that no further signals are enqueued after sigtimedwait. struct timespec zero_ts = absl::ToTimespec(absl::ZeroDuration()); - its.it_value = its.it_interval = zero_ts; - ASSERT_NO_ERRNO(timer.Set(0, its)); - - // The timer should have sent only a single signal, even though the kernel - // supports enqueueing of multiple RT signals. siginfo_t si; ASSERT_THAT(sigtimedwait(&mask, &si, &zero_ts), SyscallSucceedsWithValue(kSigno)); EXPECT_EQ(si.si_signo, kSigno); EXPECT_EQ(si.si_code, SI_TIMER); EXPECT_EQ(si.si_timerid, timer.get()); - // si_overrun was reset by timer_settime. - EXPECT_EQ(si.si_overrun, 0); + EXPECT_EQ(si.si_overrun, kCycles - 1); EXPECT_EQ(si.si_int, kSigvalue); + + // Stop the timer so that no further signals are enqueued after sigtimedwait. + its.it_value = its.it_interval = zero_ts; + ASSERT_NO_ERRNO(timer.Set(0, its)); + + // The timer should have sent only a single signal, even though the kernel + // supports enqueueing of multiple RT signals. EXPECT_THAT(sigtimedwait(&mask, &si, &zero_ts), SyscallFailsWithErrno(EAGAIN)); } @@ -574,65 +580,6 @@ TEST(IntervalTimerTest, AlreadyPendingSignal) { sigtimedwait(&mask, &si, &zero_ts); } -TEST(IntervalTimerTest, IgnoredSignalCountsAsOverrun) { - constexpr int kSigno = SIGUSR1; - constexpr int kSigvalue = 42; - - // Ignore kSigno. - struct sigaction sa = {}; - sa.sa_handler = SIG_IGN; - const auto scoped_sigaction = - ASSERT_NO_ERRNO_AND_VALUE(ScopedSigaction(kSigno, sa)); - - // Unblock kSigno so that ignored signals will be discarded. - sigset_t mask; - sigemptyset(&mask); - sigaddset(&mask, kSigno); - auto scoped_sigmask = - ASSERT_NO_ERRNO_AND_VALUE(ScopedSignalMask(SIG_UNBLOCK, mask)); - - struct sigevent sev = {}; - sev.sigev_notify = SIGEV_THREAD_ID; - sev.sigev_signo = kSigno; - sev.sigev_value.sival_int = kSigvalue; - sev.sigev_notify_thread_id = gettid(); - auto timer = ASSERT_NO_ERRNO_AND_VALUE(TimerCreate(CLOCK_MONOTONIC, sev)); - - constexpr absl::Duration kPeriod = absl::Seconds(1); - constexpr int kCycles = 3; - struct itimerspec its = {}; - its.it_value = its.it_interval = absl::ToTimespec(kPeriod); - ASSERT_NO_ERRNO(timer.Set(0, its)); - - // End the sleep one cycle short; we will sleep for one more cycle below. - absl::SleepFor(kPeriod * (kCycles - 1)); - - // Block kSigno so that ignored signals will be enqueued. - scoped_sigmask.Release()(); - scoped_sigmask = ASSERT_NO_ERRNO_AND_VALUE(ScopedSignalMask(SIG_BLOCK, mask)); - - // Sleep for 1 more cycle to give the timer time to send a signal. - absl::SleepFor(kPeriod + kTimerSlack); - - // At least kCycles expirations should have occurred, resulting in kCycles-1 - // overruns (the last expiration sent the signal successfully). - siginfo_t si; - struct timespec zero_ts = absl::ToTimespec(absl::ZeroDuration()); - ASSERT_THAT(sigtimedwait(&mask, &si, &zero_ts), - SyscallSucceedsWithValue(kSigno)); - EXPECT_EQ(si.si_signo, kSigno); - EXPECT_EQ(si.si_code, SI_TIMER); - EXPECT_EQ(si.si_timerid, timer.get()); - EXPECT_GE(si.si_overrun, kCycles - 1); - EXPECT_EQ(si.si_int, kSigvalue); - - // Kill the timer, then drain any additional signal it may have enqueued. We - // can't do this before the preceding sigtimedwait because stopping or - // deleting the timer resets si_overrun to 0. - timer.reset(); - sigtimedwait(&mask, &si, &zero_ts); -} - } // namespace } // namespace testing } // namespace gvisor