From 03e02cb10fc0cc5a6e1fc19fa29bf64b5630eea0 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Mon, 19 Dec 2022 16:22:27 -0800 Subject: [PATCH] Remove custom `alarm` impl in favor of lower level itimer syscalls These alarms/signals will now fire if when waiting on a lock (i.e. they don't depend on the event loops running). Fixes: #12415 --- src/library.js | 43 +++++-- src/shell.js | 4 + system/include/emscripten/threading.h | 2 - system/lib/libc/emscripten_syscall_stubs.c | 2 - .../libc/musl/arch/emscripten/bits/syscall.h | 2 - .../libc/musl/arch/emscripten/syscall_arch.h | 2 - system/lib/libc/musl/src/sched/sched_yield.c | 4 +- system/lib/libc/musl/src/signal/getitimer.c | 11 ++ system/lib/libc/musl/src/signal/setitimer.c | 73 +++++++++++ system/lib/pthread/emscripten_futex_wait.c | 7 +- system/lib/pthread/emscripten_yield.c | 15 ++- system/lib/pthread/library_pthread_stub.c | 18 ++- system/lib/pthread/threading_internal.h | 10 ++ test/other/test_itimer.c | 119 ++++++++++++++++++ test/other/test_itimer.out | 1 + test/test_other.py | 13 ++ tools/deps_info.py | 3 +- tools/system_libs.py | 2 +- 18 files changed, 299 insertions(+), 32 deletions(-) create mode 100644 test/other/test_itimer.c create mode 100644 test/other/test_itimer.out diff --git a/src/library.js b/src/library.js index 611ff5d688210..b1ceada343463 100644 --- a/src/library.js +++ b/src/library.js @@ -2264,14 +2264,41 @@ mergeInto(LibraryManager.library, { return 0; }, - // http://pubs.opengroup.org/onlinepubs/000095399/functions/alarm.html - alarm__deps: ['raise', '$callUserCallback'], - alarm: function(seconds) { - setTimeout(function() { - callUserCallback(function() { - _raise({{{ cDefine('SIGALRM') }}}); - }); - }, seconds*1000); + $timers: {}, + + // Helper function for setitimer that registers timers with the eventloop. + // Timers always fire on the main thread, either directly from JS (here) or + // or when the main thread is busy waiting calling _emscripten_yield. + _setitimer_js__sig: 'iid', + _setitimer_js__proxy: 'sync', + _setitimer_js__deps: ['$timers', '$callUserCallback', + '_emscripten_timeout', 'emscripten_get_now'], + _setitimer_js: function(which, timeout_ms) { +#if RUNTIME_DEBUG + dbg('setitimer_js ' + which + ' timeout=' + timeout_ms); +#endif + // First, clear any existing timer. + if (timers[which]) { + clearTimeout(timers[which].id); + delete timers[which]; + } + + // A timeout of zero simply cancels the current timeout so we have nothing + // more to do. + if (!timeout_ms) return 0; + + var id = setTimeout(() => { +#if ASSERTIONS + assert(which in timers); +#endif + delete timers[which]; +#if RUNTIME_DEBUG + dbg('itimer fired: ' + which); +#endif + callUserCallback(() => __emscripten_timeout(which, _emscripten_get_now())); + }, timeout_ms); + timers[which] = { id: id, timeout_ms: timeout_ms }; + return 0; }, // Helper for raise() to avoid signature mismatch failures: diff --git a/src/shell.js b/src/shell.js index 3f41efd0c2a07..c5b8773c24cec 100644 --- a/src/shell.js +++ b/src/shell.js @@ -318,6 +318,10 @@ if (ENVIRONMENT_IS_SHELL) { setTimeout(() => onload(readBinary(f)), 0); }; + if (typeof clearTimeout == 'undefined') { + globalThis.clearTimeout = (id) => {}; + } + if (typeof scriptArgs != 'undefined') { arguments_ = scriptArgs; } else if (typeof arguments != 'undefined') { diff --git a/system/include/emscripten/threading.h b/system/include/emscripten/threading.h index cd06a1e5ff91f..cbf8d1ef4871d 100644 --- a/system/include/emscripten/threading.h +++ b/system/include/emscripten/threading.h @@ -289,8 +289,6 @@ void emscripten_check_blocking_allowed(void); // Experimental API for syncing loaded code between pthreads. void _emscripten_thread_sync_code(); -void _emscripten_yield(); - #ifdef __cplusplus } #endif diff --git a/system/lib/libc/emscripten_syscall_stubs.c b/system/lib/libc/emscripten_syscall_stubs.c index 6152a8424db8c..3905bf0855950 100644 --- a/system/lib/libc/emscripten_syscall_stubs.c +++ b/system/lib/libc/emscripten_syscall_stubs.c @@ -254,8 +254,6 @@ UNIMPLEMENTED(pipe2, (intptr_t fds, int flags)) UNIMPLEMENTED(pselect6, (int nfds, intptr_t readfds, intptr_t writefds, intptr_t exceptfds, intptr_t timeout, intptr_t sigmaks)) UNIMPLEMENTED(recvmmsg, (int sockfd, intptr_t msgvec, size_t vlen, int flags, ...)) UNIMPLEMENTED(sendmmsg, (int sockfd, intptr_t msgvec, size_t vlen, int flags, ...)) -UNIMPLEMENTED(setitimer, (int which, intptr_t new_value, intptr_t old_value)) -UNIMPLEMENTED(getitimer, (int which, intptr_t old_value)) UNIMPLEMENTED(shutdown, (int sockfd, int how, int dummy, int dummy2, int dummy3, int dummy4)) UNIMPLEMENTED(socketpair, (int domain, int type, int protocol, intptr_t fds, int dummy, int dummy2)) UNIMPLEMENTED(wait4,(int pid, intptr_t wstatus, int options, int rusage)) diff --git a/system/lib/libc/musl/arch/emscripten/bits/syscall.h b/system/lib/libc/musl/arch/emscripten/bits/syscall.h index f88e03e6b90a0..5e7ef67eaba64 100644 --- a/system/lib/libc/musl/arch/emscripten/bits/syscall.h +++ b/system/lib/libc/musl/arch/emscripten/bits/syscall.h @@ -20,8 +20,6 @@ #define SYS_fchmod __syscall_fchmod #define SYS_getpriority __syscall_getpriority #define SYS_setpriority __syscall_setpriority -#define SYS_setitimer __syscall_setitimer -#define SYS_getitimer __syscall_getitimer #define SYS_wait4 __syscall_wait4 #define SYS_setdomainname __syscall_setdomainname #define SYS_uname __syscall_uname diff --git a/system/lib/libc/musl/arch/emscripten/syscall_arch.h b/system/lib/libc/musl/arch/emscripten/syscall_arch.h index f398401a0bcb0..5b463fd83235e 100644 --- a/system/lib/libc/musl/arch/emscripten/syscall_arch.h +++ b/system/lib/libc/musl/arch/emscripten/syscall_arch.h @@ -38,8 +38,6 @@ int __syscall_fchmod(int fd, int mode); int __syscall_getpriority(int which, int who); int __syscall_setpriority(int which, int who, int prio); int __syscall_socketcall(int call, intptr_t args); -int __syscall_setitimer(int which, intptr_t new_value, intptr_t old_value); -int __syscall_getitimer(int which, intptr_t old_value); int __syscall_wait4(int pid, intptr_t wstatus, int options, int rusage); int __syscall_setdomainname(intptr_t name, size_t size); int __syscall_uname(intptr_t buf); diff --git a/system/lib/libc/musl/src/sched/sched_yield.c b/system/lib/libc/musl/src/sched/sched_yield.c index c9e6968fd0fcf..7bd3846fc239e 100644 --- a/system/lib/libc/musl/src/sched/sched_yield.c +++ b/system/lib/libc/musl/src/sched/sched_yield.c @@ -2,7 +2,9 @@ #include "syscall.h" #if __EMSCRIPTEN__ +#include #include +#include "threading_internal.h" #endif int sched_yield() @@ -11,7 +13,7 @@ int sched_yield() // SharedArrayBuffer and wasm threads do not support explicit yielding. // For now we at least call `emscripten_yield` which processes the event queue // (along with other essential tasks). - _emscripten_yield(); + _emscripten_yield(emscripten_get_now()); return 0; #else return syscall(SYS_sched_yield); diff --git a/system/lib/libc/musl/src/signal/getitimer.c b/system/lib/libc/musl/src/signal/getitimer.c index 36d1eb9dc6e99..251b2064fa202 100644 --- a/system/lib/libc/musl/src/signal/getitimer.c +++ b/system/lib/libc/musl/src/signal/getitimer.c @@ -1,8 +1,18 @@ #include #include "syscall.h" +#ifdef __EMSCRIPTEN__ +#include +void __getitimer(int which, struct itimerval *old, double now); +#endif + int getitimer(int which, struct itimerval *old) { +#ifdef __EMSCRIPTEN__ + if (which > ITIMER_PROF) return EINVAL; + __getitimer(which, old, emscripten_get_now()); + return 0; +#else if (sizeof(time_t) > sizeof(long)) { long old32[4]; int r = __syscall(SYS_getitimer, which, old32); @@ -15,4 +25,5 @@ int getitimer(int which, struct itimerval *old) return __syscall_ret(r); } return syscall(SYS_getitimer, which, old); +#endif } diff --git a/system/lib/libc/musl/src/signal/setitimer.c b/system/lib/libc/musl/src/signal/setitimer.c index 0dfbeb4db5a7a..ec90635b3bd66 100644 --- a/system/lib/libc/musl/src/signal/setitimer.c +++ b/system/lib/libc/musl/src/signal/setitimer.c @@ -4,8 +4,80 @@ #define IS32BIT(x) !((x)+0x80000000ULL>>32) +#ifdef __EMSCRIPTEN__ +#include +#include +#include + +// Timeouts can either fire directly from the JS event loop (which calls +// `_emscripten_timeout`), or from `_emscripten_check_timers` (which is called +// from `_emscripten_yield`). In order to be able to check the timers here we +// cache the current timeout and interval for each the 3 types of timer +// (ITIMER_PROF/ITIMER_VIRTUAL/ITIMER_REAL). +static double current_timeout_ms[3]; +static double current_intervals_ms[3]; + +#define MAX(a,b) ((a)>(b)?(a):(b)) + +int _setitimer_js(int which, double timeout); + +void __getitimer(int which, struct itimerval *old, double now) +{ + double remaining_ms = MAX(current_timeout_ms[which] - now, 0); + old->it_value.tv_sec = remaining_ms / 1000; + old->it_value.tv_usec = remaining_ms * 1000; + old->it_interval.tv_sec = current_intervals_ms[which] / 1000; + old->it_interval.tv_usec = current_intervals_ms[which] * 1000; +} + +void _emscripten_timeout(int which, double now) +{ + int signum = SIGALRM; + if (which == ITIMER_PROF) + signum = SIGPROF; + else if (which == ITIMER_VIRTUAL) + signum = SIGVTALRM; + int next_timeout = current_intervals_ms[which]; + if (next_timeout) + current_timeout_ms[which] = now + next_timeout; + else + current_timeout_ms[which] = 0; + _setitimer_js(which, next_timeout); + raise(signum); +} + +void _emscripten_check_timers(double now) +{ + for (int which = 0; which < 3; which++) { + if (current_timeout_ms[which]) { + // Only call out to JS to get the current time if it was not passed in + // *and* we have one or more timers set. + if (!now) + now = emscripten_get_now(); + if (now >= current_timeout_ms[which]) + _emscripten_timeout(which, now); + } + } +} +#endif + int setitimer(int which, const struct itimerval *restrict new, struct itimerval *restrict old) { +#ifdef __EMSCRIPTEN__ + if (which > ITIMER_PROF) return EINVAL; + double now = emscripten_get_now(); + if (old) { + __getitimer(which, old, now); + } + if (new->it_value.tv_sec || new->it_value.tv_usec) { + current_timeout_ms[which] = now + new->it_value.tv_sec * 1000 + new->it_value.tv_usec / 1000; + current_intervals_ms[which] = new->it_interval.tv_sec * 1000 + new->it_interval.tv_usec / 1000; + } else { + current_timeout_ms[which] = 0; + current_intervals_ms[which] = 0; + } + return _setitimer_js(which, new->it_value.tv_sec * 1000 + new->it_value.tv_usec / 1000); +#else if (sizeof(time_t) > sizeof(long)) { time_t is = new->it_interval.tv_sec, vs = new->it_value.tv_sec; long ius = new->it_interval.tv_usec, vus = new->it_value.tv_usec; @@ -23,4 +95,5 @@ int setitimer(int which, const struct itimerval *restrict new, struct itimerval return __syscall_ret(r); } return syscall(SYS_setitimer, which, new, old); +#endif } diff --git a/system/lib/pthread/emscripten_futex_wait.c b/system/lib/pthread/emscripten_futex_wait.c index 2862412299655..a7670d9efa9e3 100644 --- a/system/lib/pthread/emscripten_futex_wait.c +++ b/system/lib/pthread/emscripten_futex_wait.c @@ -64,7 +64,7 @@ static int futex_wait_main_browser_thread(volatile void* addr, // We were told to stop waiting, so stop. break; } - _emscripten_yield(); + _emscripten_yield(now); // Check the value, as if we were starting the futex all over again. // This handles the following case: @@ -116,7 +116,10 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms) return -EINVAL; } - _emscripten_yield(); + // Pass 0 here, which means we don't have access to the current time in this + // function. This tells _emscripten_yield to call emscripten_get_now if (and + // only if) it needs to know the time. + _emscripten_yield(0); int ret; emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_WAITFUTEX); diff --git a/system/lib/pthread/emscripten_yield.c b/system/lib/pthread/emscripten_yield.c index 4c97309743ed9..e0f59cad4b111 100644 --- a/system/lib/pthread/emscripten_yield.c +++ b/system/lib/pthread/emscripten_yield.c @@ -15,14 +15,14 @@ static void dummy() { } +static void dummy2(double now) +{ +} + weak_alias(dummy, _emscripten_thread_sync_code); +weak_alias(dummy2, _emscripten_check_timers); -/* - * Called whenever a thread performs a blocking action (or calls sched_yield). - * This function takes care of running the event queue and other housekeeping - * tasks. - */ -void _emscripten_yield() { +void _emscripten_yield(double now) { int is_runtime_thread = emscripten_is_main_runtime_thread(); // When a secondary thread crashes, we need to be able to interrupt the main @@ -37,6 +37,9 @@ void _emscripten_yield() { emscripten_unwind_to_js_event_loop(); } + // This is no-op in programs that don't include use of itimer/alarm. + _emscripten_check_timers(now); + // Assist other threads by executing proxied operations that are effectively // singlethreaded. emscripten_main_thread_process_queued_calls(); diff --git a/system/lib/pthread/library_pthread_stub.c b/system/lib/pthread/library_pthread_stub.c index 4fe8e46b3ad5c..6b59872f4c798 100644 --- a/system/lib/pthread/library_pthread_stub.c +++ b/system/lib/pthread/library_pthread_stub.c @@ -46,8 +46,14 @@ void emscripten_current_thread_process_queued_calls() { // nop } -void _emscripten_yield() { - // nop +static void dummy(double now) +{ +} + +weak_alias(dummy, _emscripten_check_timers); + +void _emscripten_yield(double now) { + _emscripten_check_timers(now); } int pthread_mutex_init( @@ -403,7 +409,9 @@ void __unlock(void* ptr) {} // proper sleeps, so simulate a busy spin wait loop instead. void emscripten_thread_sleep(double msecs) { double start = emscripten_get_now(); - while (emscripten_get_now() - start < msecs) { - // Do nothing. - } + double now = start; + do { + _emscripten_yield(now); + now = emscripten_get_now(); + } while (now - start < msecs); } diff --git a/system/lib/pthread/threading_internal.h b/system/lib/pthread/threading_internal.h index f16bda2f03bc6..9b810c2e2582b 100644 --- a/system/lib/pthread/threading_internal.h +++ b/system/lib/pthread/threading_internal.h @@ -96,6 +96,16 @@ typedef struct thread_profiler_block { char name[EM_THREAD_NAME_MAX]; } thread_profiler_block; +// Called whenever a thread performs a blocking action (or calls sched_yield). +// This function takes care of running the event queue and other housekeeping +// tasks. +// +// If that caller already know the current time it can pass it vai the now +// argument. This can save _emscripten_check_timers from needing to call out to +// JS to get the current time. Passing 0 means that caller doesn't know the +// the current time. +void _emscripten_yield(double now); + void __emscripten_init_main_thread_js(void* tb); void _emscripten_thread_profiler_enable(); void __emscripten_thread_cleanup(pthread_t thread); diff --git a/test/other/test_itimer.c b/test/other/test_itimer.c new file mode 100644 index 0000000000000..40a7cd29c1923 --- /dev/null +++ b/test/other/test_itimer.c @@ -0,0 +1,119 @@ +// Copyright 2022 The Emscripten Authors. All rights reserved. +// Emscripten is available under two separate licenses, the MIT license and the +// University of Illinois/NCSA Open Source License. Both these licenses can be +// found in the LICENSE file. + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +_Atomic int got_alarm[3]; + +void alarm_handler(int dummy) { + printf("Received SIGALRM!\n"); + got_alarm[ITIMER_REAL]++; +} + +void vtalarm_handler(int dummy) { + printf("Received SIGVTALRM!\n"); + got_alarm[ITIMER_VIRTUAL]++; +} + +void prof_handler(int dummy) { + printf("Received SIGVTALRM!\n"); + got_alarm[ITIMER_PROF]++; +} + +void test_oneoff(int which) { + memset(got_alarm, 0, sizeof(got_alarm)); + + int rtn; + struct itimerval val; + memset(&val, 0, sizeof(val)); + + // Set a timer for 1 second + val.it_value.tv_sec = 1; + rtn = setitimer(which, &val, NULL); + assert(rtn == 0); + + rtn = getitimer(which, &val); + assert(rtn == 0); + printf("ms remainging: %d\n", val.it_value.tv_usec / 1000); + assert(val.it_value.tv_usec || val.it_value.tv_sec); + + // Wait 100ms + usleep(100 * 1000); + + // Verify less time remains + rtn = getitimer(which, &val); + assert(rtn == 0); + printf("ms remainging: %d\n", val.it_value.tv_usec / 1000); + assert(val.it_value.tv_sec == 0); + assert(val.it_value.tv_usec > 0); + + // Wait 1s + assert(!got_alarm[which]); + usleep(1000 * 1000); + + // Verify that the time fired and is no longer active + assert(got_alarm[which]); + rtn = getitimer(which, &val); + assert(val.it_value.tv_sec == 0); + assert(val.it_value.tv_usec == 0); +} + +#define NUM_TIMERS 10 +#define ERROR_MARGIN 3 + +void test_sequence(int which) { + memset(got_alarm, 0, sizeof(got_alarm)); + // Set a timer to fire every 100ms + struct itimerval val; + val.it_value.tv_sec = 0; + val.it_value.tv_usec = 100 * 1000; + val.it_interval.tv_sec = 0; + val.it_interval.tv_usec = 100 * 1000; + int rtn = setitimer(which, &val, NULL); + // Sleep for a little over NUM_TIMERS * 100ms + usleep((NUM_TIMERS * 100 + 50) * 1000); + printf("got %d alarms\n", got_alarm[which]); + // Normally we would expect NUM_TIMERS to fire in this time + // but leave some wiggle room for scheduling anomalies. + assert(got_alarm[which] > NUM_TIMERS - ERROR_MARGIN); + assert(got_alarm[which] < NUM_TIMERS + ERROR_MARGIN); + struct itimerval old; + val.it_value.tv_sec = 0; + val.it_value.tv_usec = 0; + rtn = setitimer(ITIMER_REAL, &val, &old); +} + +void set_handlers() { + sighandler_t rtn; + rtn = signal(SIGALRM, alarm_handler); + assert(rtn != SIG_ERR); + rtn = signal(SIGVTALRM, vtalarm_handler); + assert(rtn != SIG_ERR); + rtn = signal(SIGPROF, prof_handler); + assert(rtn != SIG_ERR); +} + +int main() { + set_handlers(); + + test_oneoff(ITIMER_REAL); + test_oneoff(ITIMER_VIRTUAL); + test_oneoff(ITIMER_PROF); + + test_sequence(ITIMER_REAL); + + printf("done\n"); + return 0; +} diff --git a/test/other/test_itimer.out b/test/other/test_itimer.out new file mode 100644 index 0000000000000..19f86f493ab11 --- /dev/null +++ b/test/other/test_itimer.out @@ -0,0 +1 @@ +done diff --git a/test/test_other.py b/test/test_other.py index 8a6220dc4c954..76ccee8436e0a 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -12794,3 +12794,16 @@ def test_signext_lowering(self): self.assertContained('--signext-lowering', err) err = self.run_process(cmd + ['-sMIN_CHROME_VERSION=73'], stderr=subprocess.PIPE).stderr self.assertContained('--signext-lowering', err) + + def test_itimer(self): + self.do_other_test('test_itimer.c') + + @node_pthreads + def test_itimer_pthread(self): + self.do_other_test('test_itimer.c') + + @node_pthreads + def test_itimer_proxy_to_pthread(self): + self.set_setting('PROXY_TO_PTHREAD') + self.set_setting('EXIT_RUNTIME') + self.do_other_test('test_itimer.c') diff --git a/tools/deps_info.py b/tools/deps_info.py index e831adccd205e..85755a5b62fb5 100644 --- a/tools/deps_info.py +++ b/tools/deps_info.py @@ -51,7 +51,8 @@ from tools.settings import settings _deps_info = { - 'alarm': ['raise'], + 'alarm': ['_emscripten_timeout'], + 'setitimer': ['_emscripten_timeout'], 'Mix_LoadWAV_RW': ['fileno'], 'SDL_CreateRGBSurface': ['malloc', 'free'], 'SDL_GL_GetProcAddress': ['malloc'], diff --git a/tools/system_libs.py b/tools/system_libs.py index 02e0570546809..820022f1aa4c8 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -966,7 +966,7 @@ def get_files(self): 'res_query.c', 'res_querydomain.c', 'proto.c', 'gethostbyaddr.c', 'gethostbyaddr_r.c', 'gethostbyname.c', 'gethostbyname2_r.c', 'gethostbyname_r.c', 'gethostbyname2.c', - 'alarm.c', 'syscall.c', 'popen.c', 'pclose.c', + 'syscall.c', 'popen.c', 'pclose.c', 'getgrouplist.c', 'initgroups.c', 'wordexp.c', 'timer_create.c', 'getentropy.c', 'getauxval.c',