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: wrong Unix error code from panic. #24284

Open
robpike opened this Issue Mar 6, 2018 · 14 comments

Comments

Projects
None yet
6 participants
@robpike
Copy link
Contributor

robpike commented Mar 6, 2018

Please answer these questions before submitting your issue. Thanks!

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

go version devel +baf3eb1625 Tue Mar 6 01:11:26 2018 +0000 darwin/amd64

% cat p.go
package main

func main() {
	panic(3)
}
% ./p
panic: 3

goroutine 1 [running]:
main.main()
	/Users/r/p.go:4 +0x39
% echo $?
2
% 

Exit code 2 traditionally means 'incorrect arguments', as in flag.Usage.

Panic should be something like 127 or 255 on Unix.

Reported by darren.e.grant@gmail.com on nuts.

@cachvico

This comment has been minimized.

Copy link

cachvico commented Mar 7, 2018

or simply 1 meaning 'general error' ?

(http://tldp.org/LDP/abs/html/exitcodes.html)

@robpike

This comment has been minimized.

Copy link
Contributor Author

robpike commented Mar 7, 2018

As has been pointed out in the mail thread, it might be best to leave this alone and say that the error that triggered this is in a program that takes exit code 2 as 'success'. That is certainly nonstandard.

@cachvico

This comment has been minimized.

Copy link

cachvico commented Mar 7, 2018

Sure, although a note in the language spec might avoid some similar confusion in the future..

I'll open an issue with Supervisor (http://supervisord.org/)

@andybons andybons added the NeedsFix label Mar 7, 2018

@andybons andybons added this to the Go1.11 milestone Mar 7, 2018

@andybons

This comment has been minimized.

Copy link
Member

andybons commented Mar 7, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Jul 10, 2018

@cachvico

This comment has been minimized.

Copy link

cachvico commented Sep 28, 2018

Thanks!

@odeke-em

This comment has been minimized.

Copy link
Member

odeke-em commented Mar 12, 2019

How's it going @cachvico? Any news from filing the bug with Supervisord?

@ianlancetaylor sure let's document in the spec that a panic returns a non-zero code to the shell.
@robpike @aclements shall we proceed with making a panic return 127 or 255? @robpike if you might have time, a reference to a UNIX manual would be useful for us to include in the commit for posterity.

@cachvico

This comment has been minimized.

Copy link

cachvico commented Mar 12, 2019

Thanks - note that the Supervisord issue should be fixed on their side as per the above linked issue.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Mar 12, 2019

Exit status 127 is used by the shell to indicate "command not found."
Exit status 126 is used by the shell to indicate "command found but not executable."
Exit status values 128 and up are used by the shell to indicate exiting due to a signal.

If a C++ program exits due to an uncaught throw it raises SIGABRT and therefore exits with status 134 (on GNU/Linux).

If a Python program exits due to raising an exception, it exits with status 1.

I think we should change an unrecovered panic to exit with status 1 rather than 2. And we should document in the language spec that an uncaught panic will cause the program to exit with a non-zero exit status, but we shouldn't document it further.

@cachvico

This comment has been minimized.

Copy link

cachvico commented Mar 14, 2019

The quickest solution to the original problem would simply be a correction to the spec at https://golang.org/pkg/builtin/#panic

I've submitted a change at https://go-review.googlesource.com/c/go/+/167709 - is this correct?

@cachvico

This comment has been minimized.

Copy link

cachvico commented Mar 14, 2019

Beg pardon - just realised the previous post said not to document the value explicitly.

May I make the change suggested by @ianlancetaylor to change the process exit code 2 to 1, and correct my change to the comment appropriately?

This would involve an edit to src/runtime/signal_sighandler.go line 151, and a supporting test-case.

@odeke-em

This comment has been minimized.

Copy link
Member

odeke-em commented Mar 17, 2019

I've submitted a change at https://go-review.googlesource.com/c/go/+/167709 - is this correct?

Thanks for submitting the change @cachvico! I've posted some comments on it.

May I make the change suggested by @ianlancetaylor to change the process exit code 2 to 1, and correct my change to the comment appropriately?
This would involve an edit to src/runtime/signal_sighandler.go line 151, and a supporting test-case.

Oh yes you can, all yours but we can review and submit those changes piece-meal/independent of the docs update.

@Gnouc

This comment has been minimized.

Copy link
Contributor

Gnouc commented Mar 17, 2019

Exit status values 128 and up are used by the shell to indicate exiting due to a signal.

@ianlancetaylor Note that only values greater than 128 are used by the shell to indicate exiting due to a signal.

Some documents indicate that 128 is used to indicate an invalid argument to shell builtin exit, but POSIX leave that behavior unspecified.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Mar 17, 2019

@Gnouc Thanks for the correction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.