-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: musl uses signal 34 for setgid #39343
Comments
According to the stack traces, it looks like the test calls the C function Separately, it looks like You can put this in a separate program by creating two files in an empty directory: x.go package cgotest
/*
#include <sys/types.h>
#include <unistd.h>
*/
import "C"
import (
"os"
"os/signal"
"syscall"
"testing"
"time"
)
func runTestSetgid() bool {
c := make(chan bool)
go func() {
C.setgid(0)
c <- true
}()
select {
case <-c:
return true
case <-time.After(5 * time.Second):
return false
}
}
func testSetgid(t *testing.T) {
if !runTestSetgid() {
t.Error("setgid hung")
}
// Now try it again after using signal.Notify.
signal.Notify(make(chan os.Signal, 1), syscall.SIGINT)
if !runTestSetgid() {
t.Error("setgid hung after signal.Notify")
}
} x_test.go: package cgotest
import "testing"
func TestSetgid(t *testing.T) {
testSetgid(t)
} What happens when you run |
Doesn't return, outputs nothing at all. Output with
Not sure if helpful but here is a |
Thanks for the strace output. What would be really helpful is to run
Right now the strace output is mixing in all the |
While doing that I noticed that this is definitely flaky sometimes the program terminates and sometimes (most of the times) it doesn't here is the |
Thanks. Looks like the C library is using signal 34 to implement I see it here in the musl sources: https://git.musl-libc.org/cgit/musl/tree/src/internal/pthread_impl.h#n117 The fix will be to change runtime/sigtab_linux_generic.go and runtime/sigtab_linux_mipsx.go to handle signal 34 the way we handle signals 32 and 33. That will have a chance of breaking some existing programs, so marking this for 1.16. |
Hello, |
@GeorgeTsilias Sure, go for it. Note that at this stage in the release process any change will not appear until the 1.16 release. Thanks. |
Change https://golang.org/cl/236518 mentions this issue: |
This was reported earlier in musl list, and their comment is at https://www.openwall.com/lists/musl/2018/10/09/3. The suggestion there was to use |
With the suggested patch, I don't see the hangs anymore. And most of the time things work. But on my application I also see sometimes gopanic with backtrace:
|
I also tested CL 236518 by backporting it to our Alpine go 1.14.3 package. When doing so the
|
@nmeum Thanks for the comments. It's best if we can keep issues focused on specific topics, so that we know when they are fixed. For problems with musl that are not related to the use of signal 34 for |
With CL 236518 applied there still seem to be occasional crashes in the
|
The fix here was not correct and will likely break when musl changes/reduces implementation-internal use of signals. See 49b017f#commitcomment-47808694 |
How else could we fix it? |
In the strace, I see
which indicates go code passed a In particular, musl does not guarantee that 32-34 are reserved for impl-internal use. We might free one or two of them up at some point in the future (the timer one is gratuitous and not needed), or even allocate them dynamically (probing for one that works) so that the implementation-internal signals work with qemu-user emulation. |
Go on Linux doesn't use the C library. It makes system calls directly. Here we are dealing with cases in which Go code is linked with C code. The C code naturally does use the C library. We're trying to make the Go code, making direct system calls, work well with that C code, using the C library. According to this issue and my looking at the source code, some versions of musl use signal 34 to implement I don't see any perfect solutions here. |
If you want to make FFI calls to C code using the C std library (or to libc itself), you can't be bypassing that runtime and changing its state out from under it, which is what happens if you manipulate the signal mask this way. What you're doing is fine in programs without C FFI, but not fine when you call to C. |
I don't see any perfect solutions here. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Started the go test suite using:
What did you expect to see?
A successful test run.
What did you see instead?
A failed test run:
Not sure if this is just a timeout problem on my hardware (i.e. if the timeout needs to be increased) or if this is some kind of deadlock. Any ideas how I could debug this further? I am unfourtunatly not farmilar with go internals but currently trying to make the entire go testsuite pass with our Alpine Linux package and this is the only remaining test which keeps failing.
The text was updated successfully, but these errors were encountered: