From eb6f2da5609cdb1f5a9d478116208cb3fab02259 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Thu, 7 Nov 2024 14:38:31 +0100 Subject: [PATCH] runtime: implement race-free signals using futexes --- GNUmakefile | 1 + builder/musl.go | 1 + compileopts/target.go | 2 + loader/goroot.go | 1 + src/internal/futex/futex.go | 74 +++++++++++ src/internal/futex/futex_darwin.c | 38 ++++++ src/internal/futex/futex_linux.c | 32 +++++ src/internal/futex/futex_linux.go | 55 ++++++++ src/runtime/runtime_unix.go | 206 +++++++++++++----------------- src/runtime/signal.c | 58 --------- 10 files changed, 291 insertions(+), 177 deletions(-) create mode 100644 src/internal/futex/futex.go create mode 100644 src/internal/futex/futex_darwin.c create mode 100644 src/internal/futex/futex_linux.c create mode 100644 src/internal/futex/futex_linux.go diff --git a/GNUmakefile b/GNUmakefile index 155f5c5d57..47fa7f6b0f 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -934,6 +934,7 @@ endif @cp -rp lib/musl/src/malloc build/release/tinygo/lib/musl/src @cp -rp lib/musl/src/mman build/release/tinygo/lib/musl/src @cp -rp lib/musl/src/math build/release/tinygo/lib/musl/src + @cp -rp lib/musl/src/misc build/release/tinygo/lib/musl/src @cp -rp lib/musl/src/multibyte build/release/tinygo/lib/musl/src @cp -rp lib/musl/src/signal build/release/tinygo/lib/musl/src @cp -rp lib/musl/src/stdio build/release/tinygo/lib/musl/src diff --git a/builder/musl.go b/builder/musl.go index a6699ad8d1..75cc4c37ed 100644 --- a/builder/musl.go +++ b/builder/musl.go @@ -127,6 +127,7 @@ var libMusl = Library{ "malloc/mallocng/*.c", "mman/*.c", "math/*.c", + "misc/*.c", "multibyte/*.c", "signal/" + arch + "/*.s", "signal/*.c", diff --git a/compileopts/target.go b/compileopts/target.go index 3dc8af02f6..f60ee30972 100644 --- a/compileopts/target.go +++ b/compileopts/target.go @@ -390,6 +390,7 @@ func defaultTarget(options *Options) (*TargetSpec, error) { "-platform_version", "macos", platformVersion, platformVersion, ) spec.ExtraFiles = append(spec.ExtraFiles, + "src/internal/futex/futex_darwin.c", "src/runtime/os_darwin.c", "src/runtime/runtime_unix.c", "src/runtime/signal.c") @@ -413,6 +414,7 @@ func defaultTarget(options *Options) (*TargetSpec, error) { spec.CFlags = append(spec.CFlags, "-mno-outline-atomics") } spec.ExtraFiles = append(spec.ExtraFiles, + "src/internal/futex/futex_linux.c", "src/runtime/runtime_unix.c", "src/runtime/signal.c") case "windows": diff --git a/loader/goroot.go b/loader/goroot.go index 20fee016bf..f61e0a1c4e 100644 --- a/loader/goroot.go +++ b/loader/goroot.go @@ -243,6 +243,7 @@ func pathsToOverride(goMinor int, needsSyscallPackage bool) map[string]bool { "internal/binary/": false, "internal/bytealg/": false, "internal/cm/": false, + "internal/futex/": false, "internal/fuzz/": false, "internal/reflectlite/": false, "internal/gclayout": false, diff --git a/src/internal/futex/futex.go b/src/internal/futex/futex.go new file mode 100644 index 0000000000..6b79b06f6e --- /dev/null +++ b/src/internal/futex/futex.go @@ -0,0 +1,74 @@ +package futex + +// Cross platform futex implementation. +// Futexes are supported on all major operating systems and on WebAssembly. +// +// For more information, see: https://outerproduct.net/futex-dictionary.html + +import ( + "sync/atomic" + "unsafe" +) + +// A futex is a way for userspace to wait with the pointer as the key, and for +// another thread to wake one or all waiting threads keyed on the same pointer. +// +// A futex does not change the underlying value, it only reads it before going +// to sleep (atomically) to prevent lost wake-ups. +type Futex struct { + atomic.Uint32 +} + +// Atomically check for cmp to still be equal to the futex value and if so, go +// to sleep. Return true if we were definitely awoken by a call to Wake or +// WakeAll, and false if we can't be sure of that. +func (f *Futex) Wait(cmp uint32) bool { + tinygo_futex_wait((*uint32)(unsafe.Pointer(&f.Uint32)), cmp) + + // We *could* detect a zero return value from the futex system call which + // would indicate we got awoken by a Wake or WakeAll call. However, this is + // what the manual page has to say: + // + // > Note that a wake-up can also be caused by common futex usage patterns + // > in unrelated code that happened to have previously used the futex + // > word's memory location (e.g., typical futex-based implementations of + // > Pthreads mutexes can cause this under some conditions). Therefore, + // > callers should always conservatively assume that a return value of 0 + // > can mean a spurious wake-up, and use the futex word's value (i.e., the + // > user-space synchronization scheme) to decide whether to continue to + // > block or not. + // + // I'm not sure whether we do anything like pthread does, so to be on the + // safe side we say we don't know whether the wakeup was spurious or not and + // return false. + return false +} + +// Like Wait, but times out after the number of nanoseconds in timeout. +// If timeout is 0, it may or may not be treated as Wait with infinite timeout. +// Therefore, make sure the timeout value is non-zero. +func (f *Futex) WaitUntil(cmp uint32, timeout uint64) { + tinygo_futex_wait_timeout((*uint32)(unsafe.Pointer(&f.Uint32)), cmp, timeout) +} + +// Wake a single waiter. +func (f *Futex) Wake() { + tinygo_futex_wake((*uint32)(unsafe.Pointer(&f.Uint32))) +} + +// Wake all waiters. +func (f *Futex) WakeAll() { + tinygo_futex_wake_all((*uint32)(unsafe.Pointer(&f.Uint32))) +} + +//export tinygo_futex_wait +func tinygo_futex_wait(addr *uint32, cmp uint32) + +//export tinygo_futex_wait_timeout +func tinygo_futex_wait_timeout(addr *uint32, cmp uint32, timeout uint64) + +//export tinygo_futex_wake +func tinygo_futex_wake(addr *uint32) + +//export tinygo_futex_wake_all +func tinygo_futex_wake_all(addr *uint32) diff --git a/src/internal/futex/futex_darwin.c b/src/internal/futex/futex_darwin.c new file mode 100644 index 0000000000..d749223efa --- /dev/null +++ b/src/internal/futex/futex_darwin.c @@ -0,0 +1,38 @@ +//go:build none + +// This file is manually included, to avoid CGo which would cause a circular +// import. + +#include + +int __ulock_wait(uint32_t operation, void *addr, uint64_t value, uint32_t timeout_us); +int __ulock_wait2(uint32_t operation, void *addr, uint64_t value, uint64_t timeout_ns, uint64_t value2); +int __ulock_wake(uint32_t operation, void *addr, uint64_t wake_value); + +// Operation code. +#define UL_COMPARE_AND_WAIT 1 + +// Flags to the operation value. +#define ULF_WAKE_ALL 0x00000100 +#define ULF_WAKE_THREAD 0x00000200 +#define ULF_NO_ERRNO 0x01000000 + +void tinygo_futex_wait(uint32_t *addr, uint32_t cmp) { + __ulock_wait(UL_COMPARE_AND_WAIT|ULF_NO_ERRNO, addr, (uint64_t)cmp, 0); +} + +void tinygo_futex_wait_timeout(uint32_t *addr, uint32_t cmp, uint64_t timeout) { + // Note: __ulock_wait2 is available since MacOS 11. + // I think that's fine, since the version before that (MacOS 10.15) is EOL + // since 2022. Though if needed, we could certainly use __ulock_wait instead + // and deal with the smaller timeout value. + __ulock_wait2(UL_COMPARE_AND_WAIT|ULF_NO_ERRNO, addr, (uint64_t)cmp, timeout, 0); +} + +void tinygo_futex_wake(uint32_t *addr) { + __ulock_wake(UL_COMPARE_AND_WAIT|ULF_NO_ERRNO, addr, 0); +} + +void tinygo_futex_wake_all(uint32_t *addr) { + __ulock_wake(UL_COMPARE_AND_WAIT|ULF_NO_ERRNO|ULF_WAKE_ALL, addr, 0); +} diff --git a/src/internal/futex/futex_linux.c b/src/internal/futex/futex_linux.c new file mode 100644 index 0000000000..a03c15758e --- /dev/null +++ b/src/internal/futex/futex_linux.c @@ -0,0 +1,32 @@ +//go:build none + +// This file is manually included, to avoid CGo which would cause a circular +// import. + +#include +#include +#include +#include + +#define FUTEX_WAIT 0 +#define FUTEX_WAKE 1 +#define FUTEX_PRIVATE_FLAG 128 + +void tinygo_futex_wait(uint32_t *addr, uint32_t cmp) { + syscall(SYS_futex, addr, FUTEX_WAIT|FUTEX_PRIVATE_FLAG, cmp, NULL, NULL, 0); +} + +void tinygo_futex_wait_timeout(uint32_t *addr, uint32_t cmp, uint64_t timeout) { + struct timespec ts = {0}; + ts.tv_sec = timeout / 1000000000; + ts.tv_nsec = timeout % 1000000000; + syscall(SYS_futex, addr, FUTEX_WAIT|FUTEX_PRIVATE_FLAG, cmp, &ts, NULL, 0); +} + +void tinygo_futex_wake(uint32_t *addr) { + syscall(SYS_futex, addr, FUTEX_WAKE|FUTEX_PRIVATE_FLAG, 1, NULL, NULL, 0); +} + +void tinygo_futex_wake_all(uint32_t *addr) { + syscall(SYS_futex, addr, FUTEX_WAKE|FUTEX_PRIVATE_FLAG, INT_MAX, NULL, NULL, 0); +} diff --git a/src/internal/futex/futex_linux.go b/src/internal/futex/futex_linux.go new file mode 100644 index 0000000000..ba6200260d --- /dev/null +++ b/src/internal/futex/futex_linux.go @@ -0,0 +1,55 @@ +package futex + +import ( + "unsafe" +) + +// Atomically check for cmp to still be equal to the futex value and if so, go +// to sleep. Return true if we were definitely awoken by a call to Wake or +// WakeAll, and false if we can't be sure of that. +func (f *Futex) Wait(cmp uint32) bool { + tinygo_futex_wait((*uint32)(unsafe.Pointer(&f.Uint32)), cmp) + + // We *could* detect a zero return value from the futex system call which + // would indicate we got awoken by a Wake or WakeAll call. However, this is + // what the manual page has to say: + // + // > Note that a wake-up can also be caused by common futex usage patterns + // > in unrelated code that happened to have previously used the futex + // > word's memory location (e.g., typical futex-based implementations of + // > Pthreads mutexes can cause this under some conditions). Therefore, + // > callers should always conservatively assume that a return value of 0 + // > can mean a spurious wake-up, and use the futex word's value (i.e., the + // > user-space synchronization scheme) to decide whether to continue to + // > block or not. + // + // I'm not sure whether we do anything like pthread does, so to be on the + // safe side we say we don't know whether the wakeup was spurious or not and + // return false. + return false +} + +// Like Wait, but times out after the number of nanoseconds in timeout. +func (f *Futex) WaitUntil(cmp uint32, timeout uint64) { + tinygo_futex_wait_timeout((*uint32)(unsafe.Pointer(&f.Uint32)), cmp, timeout) +} + +// Wake a single waiter. +func (f *Futex) Wake() { + tinygo_futex_wake((*uint32)(unsafe.Pointer(&f.Uint32)), 1) +} + +// Wake all waiters. +func (f *Futex) WakeAll() { + const maxInt32 = 0x7fff_ffff + tinygo_futex_wake((*uint32)(unsafe.Pointer(&f.Uint32)), maxInt32) +} + +//export tinygo_futex_wait +func tinygo_futex_wait(addr *uint32, cmp uint32) + +//export tinygo_futex_wait_timeout +func tinygo_futex_wait_timeout(addr *uint32, cmp uint32, timeout uint64) + +//export tinygo_futex_wake +func tinygo_futex_wake(addr *uint32, num uint32) diff --git a/src/runtime/runtime_unix.go b/src/runtime/runtime_unix.go index c4fd3285b6..55f3e4c777 100644 --- a/src/runtime/runtime_unix.go +++ b/src/runtime/runtime_unix.go @@ -3,6 +3,8 @@ package runtime import ( + "internal/futex" + "internal/task" "math/bits" "sync/atomic" "unsafe" @@ -222,46 +224,30 @@ func nanosecondsToTicks(ns int64) timeUnit { } func sleepTicks(d timeUnit) { - // When there are no signal handlers present, we can simply go to sleep. - if !hasSignals { - // timeUnit is in nanoseconds, so need to convert to microseconds here. - usleep(uint(d) / 1000) - return + var until timeUnit + if !hasScheduler { + until = ticks() + d } - if GOOS == "darwin" { - // Check for incoming signals. - if checkSignals() { - // Received a signal, so there's probably at least one goroutine - // that's runnable again. - return + for { + // Sleep for the given amount of time. + // If a signal arrived before going to sleep, or during the sleep, the + // sleep will exit early. + signalFutex.WaitUntil(0, uint64(ticksToNanoseconds(d))) + + // Check whether there was a signal before or during the call to + // WaitUntil. + if signalFutex.Swap(0) != 0 { + if checkSignals() && hasScheduler { + return + } } - // WARNING: there is a race condition here. If a signal arrives between - // checkSignals() and usleep(), the usleep() call will not exit early so - // the signal is delayed until usleep finishes or another signal - // arrives. - // There doesn't appear to be a simple way to fix this on MacOS. - - // timeUnit is in nanoseconds, so need to convert to microseconds here. - result := usleep(uint(d) / 1000) - if result != 0 { - checkSignals() - } - } else { - // Linux (and various other POSIX systems) implement sigtimedwait so we - // can do this in a non-racy way. - tinygo_wfi_mask(activeSignals) - if checkSignals() { - tinygo_wfi_unmask() + // Set duration (in next loop iteration) to the remaining time. + d = until - ticks() + if d <= 0 { return } - signal := tinygo_wfi_sleep(activeSignals, uint64(d)) - if signal >= 0 { - tinygo_signal_handler(signal) - checkSignals() - } - tinygo_wfi_unmask() } } @@ -352,21 +338,21 @@ func growHeap() bool { return true } -func init() { - // Set up a channel to receive signals into. - signalChan = make(chan uint32, 1) -} - -var signalChan chan uint32 - // Indicate whether signals have been registered. var hasSignals bool +// Futex for the signal handler. +// The value is 0 when there are no new signals, or 1 when there are unhandled +// signals and the main thread doesn't know about it yet. +// When a signal arrives, the futex value is changed to 1 and if it was 0 +// before, all waiters are awoken. +// When a wait exits, the value is changed to 0 and if it wasn't 0 before, the +// signals are checked. +var signalFutex futex.Futex + // Mask of signals that have been received. The signal handler atomically ORs // signals into this value. -var receivedSignals uint32 - -var activeSignals uint32 +var receivedSignals atomic.Uint32 //go:linkname signal_enable os/signal.signal_enable func signal_enable(s uint32) { @@ -376,7 +362,6 @@ func signal_enable(s uint32) { runtimePanicAt(returnAddress(0), "unsupported signal number") } hasSignals = true - activeSignals |= 1 << s // It's easier to implement this function in C. tinygo_signal_enable(s) } @@ -388,7 +373,6 @@ func signal_ignore(s uint32) { // receivedSignals into a uint32 array. runtimePanicAt(returnAddress(0), "unsupported signal number") } - activeSignals &^= 1 << s tinygo_signal_ignore(s) } @@ -399,20 +383,13 @@ func signal_disable(s uint32) { // receivedSignals into a uint32 array. runtimePanicAt(returnAddress(0), "unsupported signal number") } - activeSignals &^= 1 << s tinygo_signal_disable(s) } //go:linkname signal_waitUntilIdle os/signal.signalWaitUntilIdle func signal_waitUntilIdle() { - // Make sure all signals are sent on the channel. - for atomic.LoadUint32(&receivedSignals) != 0 { - checkSignals() - Gosched() - } - - // Make sure all signals are processed. - for len(signalChan) != 0 { + // Wait until signal_recv has processed all signals. + for receivedSignals.Load() != 0 { Gosched() } } @@ -430,102 +407,93 @@ func tinygo_signal_disable(s uint32) // //export tinygo_signal_handler func tinygo_signal_handler(s int32) { - // This loop is essentially the atomic equivalent of the following: + // The following loop is equivalent to the following: // - // receivedSignals |= 1 << s + // receivedSignals.Or(uint32(1) << uint32(s)) // - // TODO: use atomic.Uint32.And once we drop support for Go 1.22 instead of - // this loop. + // TODO: use this instead of a loop once we drop support for Go 1.22. for { mask := uint32(1) << uint32(s) - val := atomic.LoadUint32(&receivedSignals) - swapped := atomic.CompareAndSwapUint32(&receivedSignals, val, val|mask) + val := receivedSignals.Load() + swapped := receivedSignals.CompareAndSwap(val, val|mask) if swapped { break } } + + // Notify the main thread that there was a signal. + // This will exit the call to Wait or WaitUntil early. + if signalFutex.Swap(1) == 0 { + // Changed from 0 to 1, so there may have been a waiting goroutine. + // This could be optimized to avoid a syscall when there are no waiting + // goroutines. + signalFutex.WakeAll() + } } +// Task waiting for a signal to arrive, or nil if it is running or there are no +// signals. +var signalRecvWaiter *task.Task + //go:linkname signal_recv os/signal.signal_recv func signal_recv() uint32 { // Function called from os/signal to get the next received signal. - val := <-signalChan - checkSignals() - return val -} - -// Atomically find a signal that previously occured and send it into the -// signalChan channel. Return true if at least one signal was delivered this -// way, false otherwise. -func checkSignals() bool { - gotSignals := false for { - // Extract the lowest numbered signal number from receivedSignals. - val := atomic.LoadUint32(&receivedSignals) + val := receivedSignals.Load() if val == 0 { - // There is no signal ready to be received by the program (common - // case). - return gotSignals + // There are no signals to receive. Sleep until there are. + signalRecvWaiter = task.Current() + task.Pause() + continue } - num := uint32(bits.TrailingZeros32(val)) - // Do a non-blocking send on signalChan. - select { - case signalChan <- num: - // There was room free in the channel, so remove the signal number - // from the receivedSignals mask. - gotSignals = true - default: - // Could not send the signal number on the channel. This means - // there's still a signal pending. In that case, let it be received - // at which point checkSignals is called again to put the next one - // in the channel buffer. - return gotSignals - } + // Extract the lowest numbered signal number from receivedSignals. + num := uint32(bits.TrailingZeros32(val)) // Atomically clear the signal number from receivedSignals. - // TODO: use atomic.Uint32.Or once we drop support for Go 1.22 instead - // of this loop. + // TODO: use atomic.Uint32.And once we drop support for Go 1.22 instead + // of this loop, like so: + // + // receivedSignals.And(^(uint32(1) << num)) + // for { newVal := val &^ (1 << num) - swapped := atomic.CompareAndSwapUint32(&receivedSignals, val, newVal) + swapped := receivedSignals.CompareAndSwap(val, newVal) if swapped { break } - val = atomic.LoadUint32(&receivedSignals) + val = receivedSignals.Load() } + + return num } } -//export tinygo_wfi_mask -func tinygo_wfi_mask(active uint32) - -//export tinygo_wfi_sleep -func tinygo_wfi_sleep(active uint32, timeout uint64) int32 - -//export tinygo_wfi_wait -func tinygo_wfi_wait(active uint32) int32 - -//export tinygo_wfi_unmask -func tinygo_wfi_unmask() +// Reactivate the goroutine waiting for signals, if there are any. +// Return true if it was reactivated (and therefore the scheduler should run +// again), and false otherwise. +func checkSignals() bool { + if receivedSignals.Load() != 0 && signalRecvWaiter != nil { + runqueuePushBack(signalRecvWaiter) + signalRecvWaiter = nil + return true + } + return false +} func waitForEvents() { if hasSignals { - // We could have used pause() here, but that function is impossible to - // use in a race-free way: - // https://www.cipht.net/2023/11/30/perils-of-pause.html - // Therefore we need something better. - // Note: this is unsafe with multithreading, because sigprocmask is only - // defined for single-threaded applictions. - tinygo_wfi_mask(activeSignals) - if checkSignals() { - tinygo_wfi_unmask() - return + // Wait as long as the futex value is 0. + // This can happen either before or during the call to Wait. + // This can be optimized: if the value is nonzero we don't need to do a + // futex wait syscall and can instead immediately call checkSignals. + signalFutex.Wait(0) + + // Check for signals that arrived before or during the call to Wait. + // If there are any signals, the value is 0. + if signalFutex.Swap(0) != 0 { + checkSignals() } - signal := tinygo_wfi_wait(activeSignals) - tinygo_signal_handler(signal) - checkSignals() - tinygo_wfi_unmask() } else { // The program doesn't use signals, so this is a deadlock. runtimePanic("deadlocked: no event source") diff --git a/src/runtime/signal.c b/src/runtime/signal.c index ba4338a6d2..87af43011e 100644 --- a/src/runtime/signal.c +++ b/src/runtime/signal.c @@ -30,61 +30,3 @@ void tinygo_signal_disable(uint32_t sig) { act.sa_handler = SIG_DFL; sigaction(sig, &act, NULL); } - -// Implement waitForEvents and sleep with signals. -// Warning: sigprocmask is not defined in a multithreaded program so will need -// to be replaced with something else once we implement threading on POSIX. - -// Signals active before a call to tinygo_wfi_mask. -static sigset_t active_signals; - -static void tinygo_set_signals(sigset_t *mask, uint32_t signals) { - sigemptyset(mask); - for (int i=0; i<32; i++) { - if ((signals & (1<