Skip to content

Commit dd8c4db

Browse files
committed
Write more tests for signal handling
There's now a much stronger level of assurance that signaling on Windows will be atomic, low-latency, low tail latency, and shall never deadlock.
1 parent 0e59afb commit dd8c4db

19 files changed

+407
-75
lines changed

libc/calls/clock_nanosleep-nt.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ static textwindows int sys_clock_nanosleep_nt_impl(int clock,
4040
return 0;
4141
msdelay = timespec_tomillis(timespec_sub(abs, now));
4242
msdelay = MIN(msdelay, -1u);
43-
if (_park_norestart(msdelay, waitmask))
43+
if (_park_norestart(msdelay, waitmask) == -1)
4444
return -1;
4545
}
4646
}

libc/calls/park.c

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,17 @@
1717
│ PERFORMANCE OF THIS SOFTWARE. │
1818
╚─────────────────────────────────────────────────────────────────────────────*/
1919
#include "libc/calls/internal.h"
20+
#include "libc/calls/sig.internal.h"
2021
#include "libc/calls/struct/sigset.h"
22+
#include "libc/calls/syscall_support-nt.internal.h"
2123
#include "libc/intrin/atomic.h"
24+
#include "libc/intrin/weaken.h"
25+
#include "libc/nt/enum/wait.h"
26+
#include "libc/nt/events.h"
27+
#include "libc/nt/runtime.h"
2228
#include "libc/nt/synchronization.h"
29+
#include "libc/sysv/consts/sicode.h"
30+
#include "libc/sysv/errfuns.h"
2331
#include "libc/thread/posixthread.internal.h"
2432
#ifdef __x86_64__
2533

@@ -28,17 +36,39 @@
2836
// raises ECANCELED if this POSIX thread was canceled in masked mode
2937
textwindows static int _park_thread(uint32_t msdelay, sigset_t waitmask,
3038
bool restartable) {
31-
if (__sigcheck(waitmask, restartable) == -1)
32-
return -1;
33-
int expect = 0;
34-
atomic_int futex = 0;
3539
struct PosixThread *pt = _pthread_self();
40+
41+
// perform the wait operation
42+
intptr_t sigev;
43+
if (!(sigev = CreateEvent(0, 0, 0, 0)))
44+
return __winerr();
45+
pt->pt_event = sigev;
3646
pt->pt_blkmask = waitmask;
37-
atomic_store_explicit(&pt->pt_blocker, &futex, memory_order_release);
38-
bool32 ok = WaitOnAddress(&futex, &expect, sizeof(int), msdelay);
47+
atomic_store_explicit(&pt->pt_blocker, PT_BLOCKER_EVENT,
48+
memory_order_release);
49+
//!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!//
50+
int sig = 0;
51+
uint32_t ws = 0;
52+
if (!_is_canceled() &&
53+
!(_weaken(__sig_get) && (sig = _weaken(__sig_get)(waitmask))))
54+
ws = WaitForSingleObject(sigev, msdelay);
55+
//!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!/!//
3956
atomic_store_explicit(&pt->pt_blocker, 0, memory_order_release);
40-
if (ok && __sigcheck(waitmask, restartable) == -1)
57+
CloseHandle(sigev);
58+
59+
// recursion is now safe
60+
if (ws == -1)
61+
return __winerr();
62+
int handler_was_called = 0;
63+
if (sig)
64+
handler_was_called = _weaken(__sig_relay)(sig, SI_KERNEL, waitmask);
65+
if (_check_cancel())
4166
return -1;
67+
if (handler_was_called & SIG_HANDLED_NO_RESTART)
68+
return eintr();
69+
if (handler_was_called & SIG_HANDLED_SA_RESTART)
70+
if (!restartable)
71+
return eintr();
4272
return 0;
4373
}
4474

libc/calls/pause-nt.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,21 @@
1717
│ PERFORMANCE OF THIS SOFTWARE. │
1818
╚─────────────────────────────────────────────────────────────────────────────*/
1919
#include "libc/calls/internal.h"
20+
#include "libc/calls/struct/sigset.internal.h"
2021
#include "libc/calls/syscall_support-nt.internal.h"
2122
#ifdef __x86_64__
2223

2324
textwindows int sys_pause_nt(void) {
2425
int rc;
26+
// we don't strictly need to block signals, but it reduces signal
27+
// delivery latency, by preventing other threads from delivering a
28+
// signal asynchronously. it takes about ~5us to deliver a signal
29+
// using SetEvent() whereas it takes ~30us to use SuspendThread(),
30+
// GetThreadContext(), SetThreadContext(), and ResumeThread().
31+
BLOCK_SIGNALS;
2532
while (!(rc = _park_norestart(-1u, 0)))
2633
donothing;
34+
ALLOW_SIGNALS;
2735
return rc;
2836
}
2937

libc/calls/sig.c

Lines changed: 46 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "libc/calls/struct/siginfo.h"
2626
#include "libc/calls/struct/sigset.internal.h"
2727
#include "libc/calls/struct/ucontext.internal.h"
28+
#include "libc/calls/syscall_support-nt.internal.h"
2829
#include "libc/calls/ucontext.h"
2930
#include "libc/dce.h"
3031
#include "libc/errno.h"
@@ -135,7 +136,24 @@ static textwindows wontreturn void __sig_terminate(int sig) {
135136
TerminateThisProcess(sig);
136137
}
137138

138-
static textwindows bool __sig_start(struct PosixThread *pt, int sig,
139+
textwindows static void __sig_wake(struct PosixThread *pt, int sig) {
140+
atomic_int *blocker;
141+
blocker = atomic_load_explicit(&pt->pt_blocker, memory_order_acquire);
142+
if (!blocker)
143+
return;
144+
// threads can create semaphores on an as-needed basis
145+
if (blocker == PT_BLOCKER_EVENT) {
146+
STRACE("%G set %d's event object", sig, _pthread_tid(pt));
147+
SetEvent(pt->pt_event);
148+
return;
149+
}
150+
// all other blocking ops that aren't overlap should use futexes
151+
// we force restartable futexes to churn by waking w/o releasing
152+
STRACE("%G waking %d's futex", sig, _pthread_tid(pt));
153+
WakeByAddressSingle(blocker);
154+
}
155+
156+
textwindows static bool __sig_start(struct PosixThread *pt, int sig,
139157
unsigned *rva, unsigned *flags) {
140158
*rva = __sighandrvas[sig];
141159
*flags = __sighandflags[sig];
@@ -149,6 +167,7 @@ static textwindows bool __sig_start(struct PosixThread *pt, int sig,
149167
STRACE("enqueing %G on %d", sig, _pthread_tid(pt));
150168
atomic_fetch_or_explicit(&pt->tib->tib_sigpending, 1ull << (sig - 1),
151169
memory_order_relaxed);
170+
__sig_wake(pt, sig);
152171
return false;
153172
}
154173
if (*rva == (intptr_t)SIG_DFL) {
@@ -158,7 +177,7 @@ static textwindows bool __sig_start(struct PosixThread *pt, int sig,
158177
return true;
159178
}
160179

161-
static textwindows sigaction_f __sig_handler(unsigned rva) {
180+
textwindows static sigaction_f __sig_handler(unsigned rva) {
162181
atomic_fetch_add_explicit(&__sig.count, 1, memory_order_relaxed);
163182
return (sigaction_f)(__executable_start + rva);
164183
}
@@ -228,34 +247,15 @@ textwindows int __sig_relay(int sig, int sic, sigset_t waitmask) {
228247
return handler_was_called;
229248
}
230249

231-
// cancels blocking operations being performed by signaled thread
232-
textwindows void __sig_cancel(struct PosixThread *pt, int sig, unsigned flags) {
233-
atomic_int *blocker;
234-
blocker = atomic_load_explicit(&pt->pt_blocker, memory_order_acquire);
235-
if (!blocker) {
236-
STRACE("%G sent to %d asynchronously", sig, _pthread_tid(pt));
237-
return;
238-
}
239-
// threads can create semaphores on an as-needed basis
240-
if (blocker == PT_BLOCKER_EVENT) {
241-
STRACE("%G set %d's event object", sig, _pthread_tid(pt));
242-
SetEvent(pt->pt_event);
243-
return;
244-
}
245-
// all other blocking ops that aren't overlap should use futexes
246-
// we force restartable futexes to churn by waking w/o releasing
247-
STRACE("%G waking %d's futex", sig, _pthread_tid(pt));
248-
WakeByAddressSingle(blocker);
249-
}
250-
251250
// the user's signal handler callback is wrapped with this trampoline
252251
static textwindows wontreturn void __sig_tramp(struct SignalFrame *sf) {
253252
int sig = sf->si.si_signo;
254253
struct CosmoTib *tib = __get_tls();
255254
struct PosixThread *pt = (struct PosixThread *)tib->tib_pthread;
255+
atomic_store_explicit(&pt->pt_intoff, 0, memory_order_release);
256256
for (;;) {
257257

258-
// update the signal mask in preparation for signal handller
258+
// update the signal mask in preparation for signal handler
259259
sigset_t blocksigs = __sighandmask[sig];
260260
if (!(sf->flags & SA_NODEFER))
261261
blocksigs |= 1ull << (sig - 1);
@@ -302,12 +302,16 @@ static textwindows int __sig_killer(struct PosixThread *pt, int sig, int sic) {
302302
return 0;
303303
}
304304

305-
// we can't preempt threads that masked sig or are blocked
306-
if (atomic_load_explicit(&pt->tib->tib_sigmask, memory_order_acquire) &
307-
(1ull << (sig - 1))) {
305+
// we can't preempt threads that masked sig or are blocked. we aso
306+
// need to ensure we don't the target thread's stack if many signals
307+
// need to be delivered at once. we also need to make sure two threads
308+
// can't deadlock by killing each other at the same time.
309+
if ((atomic_load_explicit(&pt->tib->tib_sigmask, memory_order_acquire) &
310+
(1ull << (sig - 1))) ||
311+
atomic_exchange_explicit(&pt->pt_intoff, 1, memory_order_acquire)) {
308312
atomic_fetch_or_explicit(&pt->tib->tib_sigpending, 1ull << (sig - 1),
309313
memory_order_relaxed);
310-
__sig_cancel(pt, sig, flags);
314+
__sig_wake(pt, sig);
311315
return 0;
312316
}
313317

@@ -321,28 +325,26 @@ static textwindows int __sig_killer(struct PosixThread *pt, int sig, int sic) {
321325
uintptr_t th = _pthread_syshand(pt);
322326
if (atomic_load_explicit(&pt->tib->tib_sigpending, memory_order_acquire) &
323327
(1ull << (sig - 1))) {
328+
atomic_store_explicit(&pt->pt_intoff, 0, memory_order_release);
324329
return 0;
325330
}
326331

327332
// take control of thread
328333
// suspending the thread happens asynchronously
329334
// however getting the context blocks until it's frozen
330-
static pthread_spinlock_t killer_lock;
331-
pthread_spin_lock(&killer_lock);
332335
if (SuspendThread(th) == -1u) {
333336
STRACE("SuspendThread failed w/ %d", GetLastError());
334-
pthread_spin_unlock(&killer_lock);
337+
atomic_store_explicit(&pt->pt_intoff, 0, memory_order_release);
335338
return ESRCH;
336339
}
337340
struct NtContext nc;
338341
nc.ContextFlags = kNtContextFull;
339342
if (!GetThreadContext(th, &nc)) {
340343
STRACE("GetThreadContext failed w/ %d", GetLastError());
341344
ResumeThread(th);
342-
pthread_spin_unlock(&killer_lock);
345+
atomic_store_explicit(&pt->pt_intoff, 0, memory_order_release);
343346
return ESRCH;
344347
}
345-
pthread_spin_unlock(&killer_lock);
346348

347349
// we can't preempt threads that masked sig or are blocked
348350
// we can't preempt threads that are running in win32 code
@@ -354,7 +356,8 @@ static textwindows int __sig_killer(struct PosixThread *pt, int sig, int sic) {
354356
atomic_fetch_or_explicit(&pt->tib->tib_sigpending, 1ull << (sig - 1),
355357
memory_order_relaxed);
356358
ResumeThread(th);
357-
__sig_cancel(pt, sig, flags);
359+
atomic_store_explicit(&pt->pt_intoff, 0, memory_order_release);
360+
__sig_wake(pt, sig);
358361
return 0;
359362
}
360363

@@ -387,10 +390,11 @@ static textwindows int __sig_killer(struct PosixThread *pt, int sig, int sic) {
387390
nc.Rsp = sp;
388391
if (!SetThreadContext(th, &nc)) {
389392
STRACE("SetThreadContext failed w/ %d", GetLastError());
393+
atomic_store_explicit(&pt->pt_intoff, 0, memory_order_release);
390394
return ESRCH;
391395
}
392396
ResumeThread(th);
393-
__sig_cancel(pt, sig, flags);
397+
__sig_wake(pt, sig);
394398
return 0;
395399
}
396400

@@ -404,6 +408,7 @@ textwindows int __sig_kill(struct PosixThread *pt, int sig, int sic) {
404408
}
405409

406410
// sends signal to any other thread
411+
// this should only be called by non-posix threads
407412
textwindows void __sig_generate(int sig, int sic) {
408413
struct Dll *e;
409414
struct PosixThread *pt, *mark = 0;
@@ -450,6 +455,7 @@ textwindows void __sig_generate(int sig, int sic) {
450455
}
451456
_pthread_unlock();
452457
if (mark) {
458+
// no lock needed since current thread is nameless and formless
453459
__sig_killer(mark, sig, sic);
454460
_pthread_unref(mark);
455461
} else {
@@ -610,13 +616,11 @@ __msabi textwindows dontinstrument bool32 __sig_console(uint32_t dwCtrlType) {
610616
// didn't have the SA_RESTART flag, and `2`, which means SA_RESTART
611617
// handlers were called (or `3` if both were the case).
612618
textwindows int __sig_check(void) {
613-
int sig;
614-
if ((sig = __sig_get(atomic_load_explicit(&__get_tls()->tib_sigmask,
615-
memory_order_acquire)))) {
616-
return __sig_raise(sig, SI_KERNEL);
617-
} else {
618-
return 0;
619-
}
619+
int sig, res = 0;
620+
while ((sig = __sig_get(atomic_load_explicit(&__get_tls()->tib_sigmask,
621+
memory_order_acquire))))
622+
res |= __sig_raise(sig, SI_KERNEL);
623+
return res;
620624
}
621625

622626
// background thread for delivering inter-process signals asynchronously
@@ -642,7 +646,7 @@ textwindows dontinstrument static uint32_t __sig_worker(void *arg) {
642646
sigs &= ~(1ull << (sig - 1));
643647
__sig_generate(sig, SI_KERNEL);
644648
}
645-
Sleep(1);
649+
Sleep(POLL_INTERVAL_MS);
646650
}
647651
return 0;
648652
}

libc/calls/sigcheck.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,18 @@
2323
#include "libc/sysv/errfuns.h"
2424

2525
textwindows int __sigcheck(sigset_t waitmask, bool restartable) {
26-
int sig, handler_was_called;
26+
int sig, handler_was_called = 0;
2727
if (_check_cancel() == -1)
2828
return -1;
29-
if (_weaken(__sig_get) && (sig = _weaken(__sig_get)(waitmask))) {
30-
handler_was_called = _weaken(__sig_relay)(sig, SI_KERNEL, waitmask);
29+
while (_weaken(__sig_get) && (sig = _weaken(__sig_get)(waitmask))) {
30+
handler_was_called |= _weaken(__sig_relay)(sig, SI_KERNEL, waitmask);
3131
if (_check_cancel() == -1)
3232
return -1;
33-
if (handler_was_called & SIG_HANDLED_NO_RESTART)
34-
return eintr();
35-
if (handler_was_called & SIG_HANDLED_SA_RESTART)
36-
if (!restartable)
37-
return eintr();
3833
}
34+
if (handler_was_called & SIG_HANDLED_NO_RESTART)
35+
return eintr();
36+
if (handler_was_called & SIG_HANDLED_SA_RESTART)
37+
if (!restartable)
38+
return eintr();
3939
return 0;
4040
}

libc/calls/sigsuspend.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,15 @@ int sigsuspend(const sigset_t *ignore) {
5353
} else {
5454
sigset_t waitmask = ignore ? *ignore : 0;
5555
if (IsWindows() || IsMetal()) {
56+
// we don't strictly need to block signals, but it reduces signal
57+
// delivery latency, by preventing other threads from delivering a
58+
// signal asynchronously. it takes about ~5us to deliver a signal
59+
// using SetEvent() whereas it takes ~30us to use SuspendThread(),
60+
// GetThreadContext(), SetThreadContext(), and ResumeThread().
61+
BLOCK_SIGNALS;
5662
while (!(rc = _park_norestart(-1u, waitmask)))
5763
donothing;
64+
ALLOW_SIGNALS;
5865
} else {
5966
rc = sys_sigsuspend((uint64_t[2]){waitmask}, 8);
6067
}

libc/intrin/itoa16.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
╚─────────────────────────────────────────────────────────────────────────────*/
1919
#include "libc/fmt/internal.h"
2020

21-
__msabi textwindows char16_t *__itoa16(char16_t p[21], uint64_t x) {
21+
__msabi textwindows dontinstrument char16_t *__itoa16(char16_t p[21],
22+
uint64_t x) {
2223
char t;
2324
size_t a, b, i = 0;
2425
do {

libc/intrin/sig.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,8 @@ void __sig_unblock(sigset_t m) {
4444
if (IsWindows() || IsMetal()) {
4545
if (__tls_enabled) {
4646
atomic_store_explicit(&__get_tls()->tib_sigmask, m, memory_order_release);
47-
if (_weaken(__sig_check)) {
47+
if (_weaken(__sig_check))
4848
_weaken(__sig_check)();
49-
}
5049
}
5150
} else {
5251
sys_sigprocmask(SIG_SETMASK, &m, 0);

libc/intrin/sigproc.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "libc/nt/enum/pageflags.h"
3030
#include "libc/nt/files.h"
3131
#include "libc/nt/memory.h"
32+
#include "libc/nt/process.h"
3233
#include "libc/nt/runtime.h"
3334
#include "libc/nt/thunk/msabi.h"
3435
#ifdef __x86_64__
@@ -42,11 +43,16 @@ __msabi extern typeof(CreateFileMapping) *const __imp_CreateFileMappingW;
4243
__msabi extern typeof(MapViewOfFileEx) *const __imp_MapViewOfFileEx;
4344
__msabi extern typeof(SetEndOfFile) *const __imp_SetEndOfFile;
4445
__msabi extern typeof(SetFilePointer) *const __imp_SetFilePointer;
46+
__msabi extern typeof(GetEnvironmentVariable)
47+
*const __imp_GetEnvironmentVariableW;
4548

46-
__msabi textwindows char16_t *__sig_process_path(char16_t *path, uint32_t pid,
47-
int create_directories) {
49+
// Generates C:\ProgramData\cosmo\sig\x\y.pid like path
50+
__msabi textwindows dontinstrument char16_t *__sig_process_path(
51+
char16_t *path, uint32_t pid, int create_directories) {
52+
char16_t buf[3];
4853
char16_t *p = path;
49-
*p++ = 'C'; // C:\ProgramData\cosmo\sig\x\y.pid
54+
uint32_t vlen = __imp_GetEnvironmentVariableW(u"SYSTEMDRIVE", buf, 3);
55+
*p++ = vlen == 2 ? buf[0] : 'C';
5056
*p++ = ':';
5157
*p++ = '\\';
5258
*p++ = 'P';

0 commit comments

Comments
 (0)