Skip to content

Commit 07fc507

Browse files
authored
Improve behavior of interval timers in setitimer (#19638)
Alarm firing can be delayed (i.e. occur after the time it was due). When this happens the next timer is due 'interval' ms after the previous scheduled time (which in case of delay is sooner then 'interval' ms after the time it actually fired). These delays (seen when tests run under load) were causing flakiness in test_itimer on bots; in particularly the test_sequence test. There are 2 things that can still go wrong with test_itimer: 1) A single timer event can be delayed long enough to cause test_oneoff to fail (this happens if the delay is > 100ms in the current version of the test). I have seen this recently on the bots as well. This PR increases the sleep time while waiting for the signal, but it could still fail under heavy load. 2) If a delay in timer firing lasts longer than the interval itself, then a timer event will be missed entirely. This could also cause the test to fail.
1 parent fe0db79 commit 07fc507

File tree

2 files changed

+25
-10
lines changed

2 files changed

+25
-10
lines changed

system/lib/libc/musl/src/signal/setitimer.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@
55
#define IS32BIT(x) !((x)+0x80000000ULL>>32)
66

77
#ifdef __EMSCRIPTEN__
8+
#include <emscripten/emscripten.h>
89
#include <signal.h>
10+
#include <stdint.h>
911
#include <stdio.h>
10-
#include <emscripten/emscripten.h>
1112

1213
#include "emscripten_internal.h"
1314

@@ -37,11 +38,25 @@ void _emscripten_timeout(int which, double now)
3738
signum = SIGPROF;
3839
else if (which == ITIMER_VIRTUAL)
3940
signum = SIGVTALRM;
40-
int next_timeout = current_intervals_ms[which];
41-
if (next_timeout)
42-
current_timeout_ms[which] = now + next_timeout;
43-
else
41+
double next_timeout = 0.0;
42+
if (current_intervals_ms[which]) {
43+
// If time went backwards, schedule the next timer as if it didn't.
44+
now = __builtin_wasm_max_f64(now, current_timeout_ms[which]);
45+
// The next alarm is due 'interval' ms after the previous one.
46+
// If this alarm was delayed, that is sooner than 'interval' ms
47+
// from now. The delay could even be so long that we missed the
48+
// next alarm(s) entirely. Schedule the alarm for the next
49+
// multiple of 'interval' ms from the original due time.
50+
uint64_t intervals =
51+
(uint64_t)(now - current_timeout_ms[which]) /
52+
(uint64_t)current_intervals_ms[which] +
53+
1;
54+
current_timeout_ms[which] +=
55+
intervals * current_intervals_ms[which];
56+
next_timeout = current_timeout_ms[which] - now;
57+
} else {
4458
current_timeout_ms[which] = 0;
59+
}
4560
_setitimer_js(which, next_timeout);
4661
raise(signum);
4762
}

test/other/test_itimer.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ void vtalarm_handler(int dummy) {
2828
}
2929

3030
void prof_handler(int dummy) {
31-
printf("Received SIGVTALRM!\n");
31+
printf("Received SIGPROF!\n");
3232
got_alarm[ITIMER_PROF]++;
3333
}
3434

@@ -46,7 +46,7 @@ void test_oneoff(int which) {
4646

4747
rtn = getitimer(which, &val);
4848
assert(rtn == 0);
49-
printf("ms remainging: %d\n", val.it_value.tv_usec / 1000);
49+
printf("ms remaining: %d\n", val.it_value.tv_usec / 1000);
5050
assert(val.it_value.tv_usec || val.it_value.tv_sec);
5151

5252
// Wait 100ms
@@ -55,13 +55,13 @@ void test_oneoff(int which) {
5555
// Verify less time remains
5656
rtn = getitimer(which, &val);
5757
assert(rtn == 0);
58-
printf("ms remainging: %d\n", val.it_value.tv_usec / 1000);
58+
printf("ms remaining: %d\n", val.it_value.tv_usec / 1000);
5959
assert(val.it_value.tv_sec == 0);
6060
assert(val.it_value.tv_usec > 0);
6161

62-
// Wait 1s
62+
// Wait 1.5s
6363
assert(!got_alarm[which]);
64-
usleep(1000 * 1000);
64+
usleep(1500 * 1000);
6565

6666
// Verify that the time fired and is no longer active
6767
assert(got_alarm[which]);

0 commit comments

Comments
 (0)