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

Jump to bottom

os/signal: Reset doesn't reset ignored signals #46321

Open
perillo opened this issue May 22, 2021 · 4 comments
Open

os/signal: Reset doesn't reset ignored signals #46321

perillo opened this issue May 22, 2021 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@perillo
Copy link
Contributor

perillo commented May 22, 2021

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

$ go version
go version go1.16.4 linux/amd64

Does this issue reproduce with the latest release?

Yes, including go1.17 devel

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOCACHE="/home/manlio/.cache/go-build"
GOENV="/home/manlio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE="*.local"
GOMODCACHE="/home/manlio/.local/lib/go/pkg/mod"
GONOPROXY=""
GONOSUMDB="*.local"
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.4"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3881295983=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.16.4 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.16.4
uname -sr: Linux 5.12.5-arch1-1
/usr/lib/libc.so.6: GNU C Library (GNU libc) release release version 2.33.
gdb --version: GNU gdb (GDB) 10.2

What did you do?

https://play.golang.org/p/vYlTzluIuBL

What did you expect to see?

The last line of the output being false

What did you see instead?

The last line is true.

See https://groups.google.com/g/golang-nuts/c/W5NNNC0h5Zc for a discussion.
Thanks to Brian Candler for the simplified example.
@perillo
Copy link
Contributor Author

perillo commented May 22, 2021

https://golang.org/cl/3580 described signal.Reset as "ability to restore initial signal handlers".

@mknyszek mknyszek added this to the Backlog milestone May 24, 2021
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 24, 2021
@mknyszek
Copy link
Contributor

CC @ianlancetaylor via https://dev.golang.org/owners

To be clear, Reset says it specifically undoes the effects of Notify (https://golang.org/pkg/os/signal/#Reset) but it does not mention Ignore. But it does also say "all signal handlers are reset," so I see your point. I worry that maybe this has been working this way long enough that we can't actually change it at this point.

@ianlancetaylor
Copy link
Contributor

At a quick look Reset already undoes a call to Ignore, except that it fails to adjust the data checked by Ignored. If I'm right that just seems like a bug, with no serious compatibility issues.

@bluec0re
Copy link

bluec0re commented Jul 6, 2021 •
edited

According to my tests (and a real life bug) this is not just with syscall.Ignored:

https://play.golang.org/p/NQlbXjitE17

The last run of sleepAndKill should have a similar output to the first run. I can't use time.Now in the playground, otherwise a measured delay would show ~2s for the last 2 executions instead of only for the one SIGINT is ignored.

EDIT: this is particular bad as ignored signals are inherited by child processes, so unmasking them before forking the child doesn't work. This can lead to quite unexpected behavior, especially if default signals like SIGCHLD are masked.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@Andriamanitra Andriamanitra mentioned this issue Oct 31, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Go Compiler / Runtime
Status: Triage Backlog
Milestone
Backlog
Development

No branches or pull requests
5 participants
@bluec0re @mknyszek @ianlancetaylor @perillo @gopherbot