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: Prevent developers from catching SIGKILL and SIGSTOP #9463

Closed
tobegit3hub opened this Issue Dec 28, 2014 · 7 comments

Comments

Projects
None yet
5 participants
@tobegit3hub
Copy link

tobegit3hub commented Dec 28, 2014

Refer to https://en.wikipedia.org/wiki/Unix_signal#SIGKILL, we know that SIGKILL and SIGSTOP can't be caught or ignored. So golang should prevent developers from trying to do that.

Now the following code is possible to compile but totally invalid.

c := make(chan os.Signal, 1)
signal.Notify(c, syscall.SIGKILL, syscall.SIGSTOP)
@davecheney

This comment has been minimized.

Copy link
Contributor

davecheney commented Dec 28, 2014

What should signal.Notify do when asked to notify on an uncatchable signal. The method does not return an error, and panic would be incompatible with existing code which is happy that those signals are uncatchable.

@minux

This comment has been minimized.

Copy link
Member

minux commented Dec 28, 2014

the channel c just won't receive any notifications, and i think that's enough.
panic will make it incompatible with existing code, for example, might just
list all the signals there.

@minux minux closed this Dec 28, 2014

@tobegit3hub

This comment has been minimized.

Copy link
Author

tobegit3hub commented Dec 28, 2014

How about a compile error?

It could be the potential problem when developers aren't aware of it.

@cespare

This comment has been minimized.

Copy link
Contributor

cespare commented Dec 28, 2014

That cannot be a compile error. The function is being called with correctly typed arguments. The compiler does not enforce library API usage concerns.

In any case, making this an error of any kind is a backwards incompatible change, as was explained by @davecheney and @minux already. Such changes are disallowed by the Go 1 compatibility promise.

More in the realm of possibility is adding go vet checks.

@tobegit3hub

This comment has been minimized.

Copy link
Author

tobegit3hub commented Dec 28, 2014

Thanks @cespare and the suggestion is nice 😃

@tobegit3hub

This comment has been minimized.

Copy link
Author

tobegit3hub commented Dec 28, 2014

It's not a big deal and every developer should know about it actually. But there's alway someone asking the question like "why can't I catch it even though I have add it in Notify". Maybe go vet or Go 2 could help to avoid the incorrect usage of golang. BTW, I can't open an issue in golang/tools and please help.

@minux

This comment has been minimized.

Copy link
Member

minux commented Dec 28, 2014

Issues about the whole Go project should be filed here.
(Including golang/go, golang/tools, and other sub-repos.)

@mikioh mikioh changed the title signal: Prevent developers from catching SIGKILL and SIGSTOP os/signal: Prevent developers from catching SIGKILL and SIGSTOP Dec 31, 2014

@golang golang locked and limited conversation to collaborators Jun 25, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.