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: check use of osyield in signal handlers #52672

Open
rhysh opened this issue May 3, 2022 · 2 comments
Open

runtime: check use of osyield in signal handlers #52672

rhysh opened this issue May 3, 2022 · 2 comments
Assignees
Labels
NeedsInvestigation
Milestone

Comments

@rhysh
Copy link
Contributor

@rhysh rhysh commented May 3, 2022

The SIGPROF handler uses runtime.osyield to spin while waiting for exclusive access to the profiling buffer. In the discussion of https://go.dev/cl/400795, @mknyszek questioned whether that was safe. I'm not sure that it is.

I see that on many OSes (Linux, FreeBSD, NetBSD, OpenBSD, Solaris, Dragonfly) that function does a sched_yield(2) syscall, which is not one of the syscalls listed in signal-safety(7) as being required by POSIX to be safe to call from signal handlers.

On top of that, the change in Go 1.18 to use timer_create(2) on Linux for CPU profiling means that many SIGPROF deliveries are now thread-targeted (versus setitimer(2)'s process-targeted signals), which may have opened the possibility of a process handling multiple overlapping SIGPROF deliveries (on separate threads).

Let's make sure the runtime.osyield calls are safe in that context, and write down why.

Here's some of the code in question: https://github.com/golang/go/blob/go1.18.1/src/runtime/cpuprof.go#L123-L129

@dr2chase dr2chase added the NeedsInvestigation label May 3, 2022
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 3, 2022

Pure system calls are basically always async-signal-safe.

POSIX doesn't require sched_yield to be async-signal-safe because POSIX doesn't require threads to be implemented in the kernel. But for those cases where osyield is just a system call, we are fine.

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented May 3, 2022

Thanks Ian. A bunch of platforms do call sched_yield through libc (or some other shared library). Presumably that's done just because we do everything through libc on those platforms, and really it's just a simple syscall under the hood. I suppose we just have to trust that and document it.

@mknyszek mknyszek self-assigned this May 3, 2022
@mknyszek mknyszek added this to the Gccgo milestone May 3, 2022
@mknyszek mknyszek removed this from the Gccgo milestone May 3, 2022
@mknyszek mknyszek added this to the Backlog milestone May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

4 participants