runtime: deliver signals under the threads scheduler when blocked on I/O

[0pcom]

Problem

Under the threads scheduler (the default on Linux/macOS), a program blocked purely on I/O, channels or mutexes never observes an OS signal. For example, a server that does signal.Notify(c, os.Interrupt); <-c while its goroutines are blocked on network I/O ignores Ctrl+C indefinitely and has to be killed.

The cause: checkSignals() — which resumes the parked os/signal signal_recv goroutine — is only ever reached from sleepTicks() (in runtime_unix.go). So a signal is only noticed while some goroutine happens to be inside time.Sleep. time.NewTicker/time.After go through the timer queue (timerRunner), not sleepTicks, so a program that blocks on I/O/channels can ignore SIGINT indefinitely.

The cooperative and multicore schedulers don't have this problem because they call checkSignals() from their idle loop (waitForEvents). The threads scheduler has no such loop, so nothing consumes signalFutex and resumes signal_recv.

Fix

Start a dedicated signal-watcher thread the first time a signal is enabled, gated to the threads scheduler (!hasScheduler && hasParallelism, which is true only there). It blocks on signalFutex and calls checkSignals() on wake — mirroring the signal half of waitForEvents(). It is a compile-time no-op for every other scheduler: the cooperative/cores schedulers already handle signals from their idle loop, and the none scheduler has no goroutines.

Testing

A channel/Accept-blocked program with no time.Sleep anywhere ignores SIGINT before this change and exits cleanly after it. Also verified against a real network daemon that blocks on I/O: SIGINT now triggers graceful shutdown, both idle and under load.

[deadprogram]

This is called as a Go routine with no exit condition. I know that waitForEvents() already has the same issue, but it would be pretty nice to have a way for a cleaner exit.

[0pcom]

Thanks for the review — addressed in 7243c59.

You're right that a goroutine with no exit condition isn't great, so the watcher now has a lifetime rather than running forever. It exists only to serve enabled signals, so that is what bounds it: enabledSignals tracks the set os/signal currently wants delivered, the last signal_disable/signal_ignore stops the thread, and a later signal_enable starts a fresh one.

The shutdown itself sets a flag, bumps the futex value, and wakes it. The bump matters as much as the wake — Wait(0) returns immediately when the futex is already non-zero, which closes the window between the store and a watcher that is just about to sleep. On the way out the watcher resets the futex to 0 so the next one can block on it.

Two details worth flagging:

• I used CAS loops rather than atomic.Uint32.Or/.And, since those methods are newer than some of the Go versions TinyGo builds against.
• I left waitForEvents() alone. It has the same shape, but it's the scheduler's own idle loop rather than a thread this PR introduces, so bounding it feels like a separate change — happy to take it on if you'd like it in scope.

Verified with a program that blocks on a channel and never on time.Sleep, so delivery can only come through the watcher: the signal arrives, signal.Stop lets the thread exit, and a later signal.Notify starts a new watcher that delivers again.

[0pcom]

Correction to my previous comment: I said I'd used CAS loops because atomic.Uint32.Or/.And are newer than some Go versions TinyGo builds against. That was wrong — this very file already uses receivedSignals.Or(...) and receivedSignals.And(...) on the same type, and TinyGo tracks recent Go.

Amended in e626202 to use Or/And, which matches the surrounding code and drops the CAS boilerplate. Behaviour is unchanged; re-verified with the same test (blocks on a channel, never time.Sleep, so delivery can only come through the watcher — signal arrives, signal.Stop lets the thread exit, later signal.Notify starts a new watcher that delivers again).

One readability note on the stop path: And returns the value before the mask is applied, so the code clears the bit from that result to get what remains enabled.

[deadprogram]

I think that this should be WakeAll() here

[deadprogram]

Here is a comment from an automated code review:

signalWatcherStarted, signalWatcherStop, and enabledSignals form a state machine that is only coherent because os/signal serializes Notify/Stop/Ignore under handlers.Lock(). The watcher's wake isn't covered by that lock, so start-after-stop can transiently run two watchers (stop sets started=0; a following signal_enable resets signalWatcherStop to 0 and spawns a second watcher before the first has woken and read the flag). It self-heals on the next stop, but signalWatcherStop is redundant the watcher can just test enabledSignals, and own its own started flag:

func signalWatcher() {
    for enabledSignals.Load() != 0 {
        signalFutex.Wait(0)
        if signalFutex.Swap(0) != 0 {
            checkSignals()
        }
    }
    signalWatcherStarted.Store(0)
}

That drops one global, removes the flag-reset race, and makes the exit condition impossible to get stale. Both flags also want atomic.Bool rather than atomic.Uint32.

[0pcom]

Both review comments addressed.

WakeAll() here — you were right, and it is a correctness bug rather than a style point. The watcher is not the only thing sleeping on signalFutex: sleepTicks waits on it at line 256 and waitForEvents at 592. Waking a single waiter can therefore wake a sleeping goroutine instead of the watcher — that goroutine consumes the value with its own Swap, and the watcher is left asleep on a futex that is back to 0, never seeing the stop flag. That is exactly the parked-thread leak stopSignalWatcher exists to prevent, so the function could fail at its one job. Changed, and it now matches what tinygo_signal_handler already does to the same futex a few lines below.

"no exit condition" — this should be resolved by stopSignalWatcher, added in e626202 after your comment. signal_disable calls it, and when the last enabled signal goes away it sets a stop flag, bumps the futex and wakes the watcher, which returns rather than parking forever. The value bump matters alongside the wake: Wait(0) returns immediately when the futex is already non-zero, which closes the window between the store and a watcher that is just about to sleep.

I left waitForEvents alone, since its lack of an exit condition predates this and is not something this PR should quietly change.

Verified with testdata/signal.go under both the default scheduler and -scheduler=threads; output matches signal.txt in both.

[deadprogram]

@0pcom I think you still need to add a test that shows this is working, such as the same current test but without the sleep and with a <-c receive.

[0pcom]

Done — testdata/signal.go now sends the signal and receives it with <-c, no sleep anywhere.

You were right that the old one proved nothing. The time.Sleep was doing the delivery rather than waiting for it: sleepTicks waits on the same futex the handler bumps and calls checkSignals on the way out, so the signal arrived on the back of the sleep whether or not anything else was working.

I checked that by disabling the watcher (commenting out the go signalWatcher()) and running both versions under -scheduler=threads:

	watcher disabled
old test, with the sleep	passes, got expected signal
new test, <-c receive	hangs, killed at 25s, no output

So the old test could not have caught this and the new one does. Blocking on the receive parks the only goroutine there is, which leaves the watcher as the only thing that can deliver.

Output is unchanged, so signal.txt stays as it is. Passes under both the default scheduler and -scheduler=threads with the watcher restored.

[deadprogram]

Thanks @0pcom, this is edited from an automated review.

I checked the new test by merging the branch onto dev and running it three ways. With -scheduler=threads and with -scheduler=tasks it passes. With go signalWatcher() removed and -scheduler=threads it hangs and I killed it at 25 seconds. So the new test does fail without the fix, which the old one could not. Thank you for that.

Three items are still open.

1. The state machine point from my earlier comment is not addressed. signalWatcherStop is still present and both flags are still atomic.Uint32. stopSignalWatcher sets started=0, then stop=1. A signal_enable that runs before the old watcher wakes sets stop=0 and finds started==0, so it starts a second watcher. Two watchers then run, and the next stop releases only one. The other parks forever, which is the leak stopSignalWatcher is there to prevent. Letting the watcher test enabledSignals and clear its own started flag removes the extra global and makes the exit condition impossible to get stale.

2. The branch is based on a dev from before the runtimePanic(Error) change. It merges cleanly, but please rebase.

3. Please shorten the comments to 2 lines at most. The block in stopSignalWatcher is 10 lines and the one in testdata/signal.go is 8. Please also remove the em dashes.

[0pcom]

All three done in a691dd8b.

signalWatcherStop is gone, signalWatcherStarted is an atomic.Bool, and the watcher tests enabledSignals and clears its own flag. I added three lines to your snippet, because clearing the flag on the way out has the mirror image of the race you found: an enable landing between the watcher's last enabledSignals.Load() and its Store(false) sees the flag still set, so it does not spawn, and then the watcher exits, leaving signals enabled with no watcher. Clearing first and rechecking closes it:

signalWatcherStarted.Store(false)
if enabledSignals.Load() == 0 || signalWatcherStarted.Swap(true) {
	return
}

I could not get either version to fail in 20000 enable/deliver/disable rounds, so that is reasoning rather than a repro. Happy to drop it for the shorter form if you prefer.

Rebased onto dev, so it now carries errUnsupportedSignal. Comments are down to two lines and the em dashes are gone.

Checked with tinygo 0.42.0 and this runtime patched into its TINYGOROOT: -scheduler=threads passes testdata/signal.go and the 20000 rounds, and hangs with go signalWatcher() commented out (killed at 30s). I could not check -scheduler=tasks locally, as it fails to link with duplicate symbol: tinygo_task_exit on the stock 0.42.0 install with no patch applied, so that looks unrelated to this branch.