Skip to content

Commit 5359b8e

Browse files
bors[bot]asomers
andauthored
Merge #1626
1626: Fix intermittency in test_timer::alarm_fires r=rtzoeller a=asomers The test was disabling the signal handler before disabling the timer. Fix intermittent failures but reversing the cleanup order. Also, speed up the timer to make the test suite complete faster. Co-authored-by: Alan Somers <[email protected]>
2 parents ee0543f + e5b9b97 commit 5359b8e

File tree

1 file changed

+13
-5
lines changed

1 file changed

+13
-5
lines changed

test/test_timer.rs

+13-5
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ fn alarm_fires() {
2323
// Avoid interfering with other signal using tests by taking a mutex shared
2424
// among other tests in this crate.
2525
let _m = crate::SIGNAL_MTX.lock();
26+
const TIMER_PERIOD: Duration = Duration::from_millis(100);
2627

2728
//
2829
// Setup
@@ -44,7 +45,7 @@ fn alarm_fires() {
4445
si_value: 0,
4546
});
4647
let mut timer = Timer::new(clockid, sigevent).expect("failed to create timer");
47-
let expiration = Expiration::Interval(Duration::from_millis(250).into());
48+
let expiration = Expiration::Interval(TIMER_PERIOD.into());
4849
let flags = TimerSetTimeFlags::empty();
4950
timer.set(expiration, flags).expect("could not set timer");
5051

@@ -73,7 +74,7 @@ fn alarm_fires() {
7374
// is never called something has gone sideways and the test fails.
7475
let starttime = Instant::now();
7576
loop {
76-
thread::sleep(Duration::from_millis(500));
77+
thread::sleep(2 * TIMER_PERIOD);
7778
if ALARM_CALLED.load(Ordering::Acquire) {
7879
break;
7980
}
@@ -82,9 +83,16 @@ fn alarm_fires() {
8283
}
8384
}
8485

85-
// Replace the old signal handler now that we've completed the test. If the
86-
// test fails this process panics, so the fact we might not get here is
87-
// okay.
86+
// Cleanup:
87+
// 1) deregister the OS's timer.
88+
// 2) Wait for a full timer period, since POSIX does not require that
89+
// disabling the timer will clear pending signals, and on NetBSD at least
90+
// it does not.
91+
// 2) Replace the old signal handler now that we've completed the test. If
92+
// the test fails this process panics, so the fact we might not get here
93+
// is okay.
94+
drop(timer);
95+
thread::sleep(TIMER_PERIOD);
8896
unsafe {
8997
sigaction(SIG, &old_handler).expect("unable to reset signal handler");
9098
}

0 commit comments

Comments
 (0)