Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix quickjs-libc signal handling race #614

Open
bnoordhuis opened this issue Oct 23, 2024 · 0 comments
Open

Fix quickjs-libc signal handling race #614

bnoordhuis opened this issue Oct 23, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@bnoordhuis
Copy link
Contributor

static uint64_t os_pending_signals;

And then:

quickjs/quickjs-libc.c

Lines 2019 to 2022 in 62f4713

static void os_signal_handler(int sig_num)
{
os_pending_signals |= ((uint64_t)1 << sig_num);
}

And lest we forget:

quickjs/quickjs-libc.c

Lines 2408 to 2410 in 62f4713

mask = (uint64_t)1 << sh->sig_num;
if (os_pending_signals & mask) {
os_pending_signals &= ~mask;

Problems:

  1. os_pending_signals is not volatile

  2. os_pending_signals is not _Atomic

  3. Signals get lost when event loop A "steals" them from event loop B

  4. The signal handler is installed with signal(2), not sigaction(2), resulting in different behavior depending on how quickjs-libc.c is compiled, at least on Linux. From man 2 signal:

The kernel's signal() system call provides System V semantics.

By default, in glibc 2 and later, the signal() wrapper function does not invoke the kernel system call. Instead, it calls sigaction(2) using flags that supply BSD semantics. This default behavior is provided as long as a suitable feature test macro is defined: _BSD_SOURCE on glibc 2.19 and earlier or _DEFAULT_SOURCE in glibc 2.19 and later. (By default, these macros are defined; see feature_test_macros(7) for details.) If such a feature test macro is not defined, then signal() provides System V semantics.

SysV semantics = handlers are re-entrant (SA_NODEFER); worse, handlers are one-shot - they are removed on delivery of a signal.

@bnoordhuis bnoordhuis added the bug Something isn't working label Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant