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

runtime: add support for os/signal #4378

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions builder/musl.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ var libMusl = Library{
"mman/*.c",
"math/*.c",
"multibyte/*.c",
"signal/" + arch + "/*.s",
"signal/*.c",
"stdio/*.c",
"string/*.c",
Expand Down
6 changes: 4 additions & 2 deletions compileopts/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,8 @@ func defaultTarget(options *Options) (*TargetSpec, error) {
)
spec.ExtraFiles = append(spec.ExtraFiles,
"src/runtime/os_darwin.c",
"src/runtime/runtime_unix.c")
"src/runtime/runtime_unix.c",
"src/runtime/signal.c")
case "linux":
spec.Linker = "ld.lld"
spec.RTLib = "compiler-rt"
Expand All @@ -411,7 +412,8 @@ func defaultTarget(options *Options) (*TargetSpec, error) {
spec.CFlags = append(spec.CFlags, "-mno-outline-atomics")
}
spec.ExtraFiles = append(spec.ExtraFiles,
"src/runtime/runtime_unix.c")
"src/runtime/runtime_unix.c",
"src/runtime/signal.c")
case "windows":
spec.Linker = "ld.lld"
spec.Libc = "mingw-w64"
Expand Down
9 changes: 9 additions & 0 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func TestBuild(t *testing.T) {
"oldgo/",
"print.go",
"reflect.go",
"signal.go",
"slice.go",
"sort.go",
"stdlib.go",
Expand Down Expand Up @@ -213,6 +214,7 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) {
// isWebAssembly := strings.HasPrefix(spec.Triple, "wasm")
isWASI := strings.HasPrefix(options.Target, "wasi")
isWebAssembly := isWASI || strings.HasPrefix(options.Target, "wasm") || (options.Target == "" && strings.HasPrefix(options.GOARCH, "wasm"))
isBaremetal := options.Target == "simavr" || options.Target == "cortex-m-qemu" || options.Target == "riscv-qemu"

for _, name := range tests {
if options.GOOS == "linux" && (options.GOARCH == "arm" || options.GOARCH == "386") {
Expand Down Expand Up @@ -277,6 +279,13 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) {
continue
}
}
if isWebAssembly || isBaremetal || options.GOOS == "windows" {
switch name {
case "signal.go":
// Signals only work on POSIX-like systems.
continue
}
}

name := name // redefine to avoid race condition
t.Run(name, func(t *testing.T) {
Expand Down
14 changes: 0 additions & 14 deletions src/os/signal/signal.go

This file was deleted.

228 changes: 226 additions & 2 deletions src/runtime/runtime_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
package runtime

import (
"math/bits"
"sync/atomic"
"unsafe"
)

Expand All @@ -12,6 +14,9 @@ func libc_write(fd int32, buf unsafe.Pointer, count uint) int
//export usleep
func usleep(usec uint) int

//export pause
func pause() int32

// void *mmap(void *addr, size_t length, int prot, int flags, int fd, off_t offset);
// Note: off_t is defined as int64 because:
// - musl (used on Linux) always defines it as int64
Expand Down Expand Up @@ -217,8 +222,47 @@ func nanosecondsToTicks(ns int64) timeUnit {
}

func sleepTicks(d timeUnit) {
// timeUnit is in nanoseconds, so need to convert to microseconds here.
usleep(uint(d) / 1000)
// 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
}

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
}

// 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()
return
}
signal := tinygo_wfi_sleep(activeSignals, uint64(d))
if signal >= 0 {
tinygo_signal_handler(signal)
checkSignals()
}
tinygo_wfi_unmask()
}
}

func getTime(clock int32) uint64 {
Expand Down Expand Up @@ -307,3 +351,183 @@ func growHeap() bool {
setHeapEnd(heapStart + heapSize)
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

// Mask of signals that have been received. The signal handler atomically ORs
// signals into this value.
var receivedSignals uint32

var activeSignals uint32

//go:linkname signal_enable os/signal.signal_enable
func signal_enable(s uint32) {
Copy link
Contributor

@leongross leongross Aug 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is a good idea to add the remaining signal functions (signal_disable, signal_ignore, signal_ignored) golang provides as well, or at least stub them out. I remember building applications relying on the signal packages and often requiring most of them.

Also functions like signal.Ignore(.) can easily be added as public API wrappers for the same functionality implemented in the runtime.

if s >= 32 {
// TODO: to support higher signal numbers, we need to turn
// receivedSignals into a uint32 array.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to do it this way uint64 might even be sufficient.
checking the values for SIGRTMAX on my system leaves me with 64. From a quick online look up is seems to be the case for many linux systems.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, uint64 is not atomic on some systems such as MIPS32.
But signal numbers higher than 31 are rarely used (I think), so we can leave that for now and implement it when someone needs it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes sense. I also had a deeper look into the standard signals on signal (7) and indeed it looks like 1-31 are the common signals which end at SIGRTMIN. So for now there are only 2 entirely user-definable signals available (SIGUSR1, SIGUSR2) but for basic signal support, this should suffice.

runtimePanicAt(returnAddress(0), "unsupported signal number")
}
hasSignals = true
activeSignals |= 1 << s
// It's easier to implement this function in C.
tinygo_signal_enable(s)
}

//go:linkname signal_ignore os/signal.signal_ignore
func signal_ignore(s uint32) {
if s >= 32 {
// TODO: to support higher signal numbers, we need to turn
// receivedSignals into a uint32 array.
runtimePanicAt(returnAddress(0), "unsupported signal number")
}
activeSignals &^= 1 << s
tinygo_signal_ignore(s)
}

//go:linkname signal_disable os/signal.signal_disable
func signal_disable(s uint32) {
if s >= 32 {
// TODO: to support higher signal numbers, we need to turn
// receivedSignals into a uint32 array.
runtimePanicAt(returnAddress(0), "unsupported signal number")
}
activeSignals &^= 1 << s
tinygo_signal_disable(s)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might also want to add signal_ignored. This could implemented via a bitmap as well.

Suggested change
func signal_ignored(s uint32)

Copy link
Member Author

@aykevl aykevl Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a code sample that requires this?
The test includes signal.Ignore(...) call, which doesn't result in signal_ignored being called.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had something in the back of my mind but I had a look at my build logs and could not find it, so let's leave it for now.

//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 {
Gosched()
}
}

//export tinygo_signal_enable
func tinygo_signal_enable(s uint32)

//export tinygo_signal_ignore
func tinygo_signal_ignore(s uint32)

//export tinygo_signal_disable
func tinygo_signal_disable(s uint32)

// void tinygo_signal_handler(int sig);
//
//export tinygo_signal_handler
func tinygo_signal_handler(s int32) {
// This loop is essentially the atomic equivalent of the following:
//
// receivedSignals |= 1 << s
//
// TODO: use atomic.Uint32.And once we drop support for Go 1.22 instead of
// this loop.
Comment on lines +437 to +438
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Go 1.22 support will not be dropped in the next view releases, we should also not consider this a blocking factor here.

Same here

for {
mask := uint32(1) << uint32(s)
val := atomic.LoadUint32(&receivedSignals)
swapped := atomic.CompareAndSwapUint32(&receivedSignals, val, val|mask)
if swapped {
break
}
}
}

//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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the return code for checkSignals has not been really checked. Is this on purpose or by accident?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used in sleepTicks, where it is used to exit early before sleeping.

gotSignals := false
for {
// Extract the lowest numbered signal number from receivedSignals.
val := atomic.LoadUint32(&receivedSignals)
if val == 0 {
// There is no signal ready to be received by the program (common
// case).
return gotSignals
}
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
}

// Atomically clear the signal number from receivedSignals.
// TODO: use atomic.Uint32.Or once we drop support for Go 1.22 instead
// of this loop.
Comment on lines +487 to +488
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Go 1.22 support will not be dropped in the next view releases, we should also not consider this a blocking factor here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, all it will do is make the code look better (and maybe a bit faster though that's probably not noticeable). I intentionally put in "Go 1.22" because when dropping support for a Go version I typically will grep for such strings to fix cases like these.

for {
newVal := val &^ (1 << num)
swapped := atomic.CompareAndSwapUint32(&receivedSignals, val, newVal)
if swapped {
break
}
val = atomic.LoadUint32(&receivedSignals)
}
}
}

//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()

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
}
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")
}
}
Loading
Loading