runtime: block signal_recv on targets without signal delivery (#5619) - #5620
Open
neomantra wants to merge 1 commit into
Open
runtime: block signal_recv on targets without signal delivery (#5619)#5620neomantra wants to merge 1 commit into
neomantra wants to merge 1 commit into
Conversation
The signalstub implementation of os/signal.signal_recv returned ^uint32(0) immediately. Upstream's os/signal.loop calls signal_recv in a tight loop with no yield point, so the watcher goroutine started by signal.Notify spun forever. On the cooperative wasm scheduler that starves every other goroutine and never returns control to the host: any program calling signal.Notify — Bubble Tea does, for example — freezes the browser tab or wasm runner at 100% CPU (a V8 profile showed 99.7% of ticks in os/signal.loop). Signals can never arrive on these targets, so block forever using deadlock(), the same primitive a blocking empty select uses. This matches the real implementation's behavior while no signal is pending. Add a behavioral test that runs on the stubbed platforms (the existing signal.go test is skipped there): before this change it times out on wasm; with it the sleep completes and the program exits. Signed-off-by: Evan Wies <evan@neomantra.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This was worked through with LLM. The text below is LLM-generated and I have read and reviewed all the code.
But I did finally get the BubbleTea List demo running in browser, compiled by TinyGo!
Summary
Make the stubbed
os/signal.signal_recvblock forever instead of returningimmediately, so
signal.Notifyno longer starves cooperative schedulers onwasm and baremetal targets.
Fixes #5619
Problem
src/runtime/signalstub.go(build tagstinygo.wasm || baremetal) stubbedsignal_recvasreturn ^uint32(0). Upstream'sos/signal.loopcallssignal_recvin a tight loop with no yield point — it relies on theruntime blocking until a signal arrives, as the POSIX implementation in
runtime_unix.godoes. With the immediate-return stub, the watchergoroutine started by
signal.Notifyspins forever.On a cooperative scheduler a spinning goroutine is always runnable, so the
scheduler never idles: on wasm,
_startnever returns to the host eventloop and the browser tab (or node/wazero) pins a core with all other
goroutines starved. Any program calling
signal.Notifyis affected —Bubble Tea does so unconditionally, which is how this surfaced (a V8
profile of the hung program showed 99.7% of ticks in
os/signal.loop).Fix
signal_recvnow callsdeadlock()— the same primitive a blocking emptyselectuses — parking the watcher goroutine forever. That matches thereal implementation's observable behavior on a system where no signal ever
arrives:
Notifysucceeds, the channel simply never receives anything, andeverything else keeps running. This is also what gc's js/wasm port does.
Verification
testdata/signalnotify.go(signal.Notify, thentime.Sleep, then printdone), registered for all platforms. Theexisting
signal.gotest is skipped on wasm/baremetal/windows, which iswhy this had no coverage.
dev@ 86d58db the wasm test hangs until the Go testtimeout kills it.
TestBuild/WebAssembly/signalnotify.goandTestBuild/Host/signalnotify.goboth pass (the host run exercises thereal POSIX
signal_recvpath, guarding both implementations).bubbles/listapp compiled forGOOS=js GOARCH=wasmpreviously froze the browser tab at 100% CPU rightafter startup; with this change it runs interactively.
deadlock()is defined by all four scheduler implementations(cooperative, threads, cores, none), covering the stub's whole build-tag
surface.
(this environment lacks the LLVM source checkout for compiler-rt); CI
covers those. If
os/signalturns out not to fit AVR flash, the test canbe excluded for AVR the way
json.go/stdlib.goare.Context
Found while verifying the large-parameter-spilling work (#5615) in a real
browser; it is independent of that change and reproduces on stock
dev.