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

os/signal: deadlock combining signal handling with syscall.AllThreadsSyscall #43149

Closed
AndrewGMorgan opened this issue Dec 12, 2020 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@AndrewGMorgan
Copy link
Contributor

What version of Go are you using (go version)?

HEAD (of 1.16 development branch)

Does this issue reproduce with the latest release?

No. It is a failure introduced with the new syscall.AllThreadsSyscall*() functions.

What operating system and processor architecture are you using (go env)?

All linux builds - with CGO_ENABLED=0

What did you do?

Following this bug: https://bugzilla.kernel.org/show_bug.cgi?id=210533 (which did not relate to the golang sources) I tried to test signal handling with the new (post 1.15) syscall.AllThreadsSyscall*() by adding the following test to the golang tree:

$ cat os/signal/signal_linux_test.go 
// +build linux

package signal

import (
        "os"
        "syscall"
        "testing"
        "time"
)

const prSetKeepCaps = 8

func TestAllThreadsSyscallSignals(t *testing.T) {
        if _, _, err := syscall.AllThreadsSyscall(syscall.SYS_PRCTL, prSetKeepCaps, 0, 0); err == syscall.ENOTSUP {
                t.Skip("AllThreadsSyscall disabled with cgo")
        }

        sig := make(chan os.Signal, 1)
        Notify(sig, os.Interrupt)

        for i := 0; i <= 100; i++ {
                if _, _, errno := syscall.AllThreadsSyscall(syscall.SYS_PRCTL, prSetKeepCaps, uintptr(i&1), 0); errno != 0 {
                        t.Fatalf("[%d] failed to set KEEP_CAPS=%d: %v", i, i&1, errno)
                }
        }

        select {
        case <-time.After(10 * time.Millisecond):
        case <-sig:
                t.Fatal("unexpected signal")
        }
        Stop(sig)
        close(sig)
}

Then in a build tree with this added test case I built and ran it as follows:

$ CGO_ENABLED=0 ../bin/go test -c os/signal
$ ./signal.test -test.v=1

What did you expect to see?

$ ./signal.test -test.v=1
=== RUN   TestAllThreadsSyscallSignals
--- PASS: TestAllThreadsSyscallSignals (0.02s)
=== RUN   TestSignal
    signal_test.go:116: sighup...
    signal_test.go:127: sigwinch...
    signal_test.go:134: sighup...
    signal_test.go:137: sighup...
--- PASS: TestSignal (0.00s)
=== RUN   TestStress
<...all good beyond here...>

What did you see instead?

$ ./signal.test -test.v=1
=== RUN   TestAllThreadsSyscallSignals
<...hangs here...>
@AndrewGMorgan
Copy link
Contributor Author

Please assign this bug to me. I have a fix.

@gopherbot
Copy link

Change https://golang.org/cl/277434 mentions this issue: os/signal: fix a deadlock with syscall.AllThreadsSyscall() use

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 12, 2020
@ianlancetaylor ianlancetaylor added this to the Go1.16 milestone Dec 12, 2020
hecg119 pushed a commit to hecg119/libcap that referenced this issue Mar 17, 2021
Avoid building any of the GO stuff:

   make GOLANG=no ...

Build with a specific build of GO:

   make GO=~/sdk/go1.16rc1/bin/go ...

Also, now golang/go#43149 is resolved in the
go1.16rc1 build (it does not work in the go1.16beta1 but we don't support
that one any more), remove the forced CGO use for the go/psx-signals build.

Signed-off-by: Andrew G. Morgan <morgan@kernel.org>
@AndrewGMorgan
Copy link
Contributor Author

For the record, #44193 does represent remaining issues with this support in 1.16.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants