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 · 15 comments
Open

runtime: wrong Unix error code from panic. #24284

robpike opened this issue Mar 6, 2018 · 15 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. early-in-cycle A change that should be done early in the 3 month dev cycle. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@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.

@LDMFD
Copy link

LDMFD commented Mar 7, 2018

or simply 1 meaning 'general error' ?

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

@robpike
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.

@LDMFD
Copy link

LDMFD 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 The path to resolution is known, but the work has not been done. label Mar 7, 2018
@andybons andybons added this to the Go1.11 milestone Mar 7, 2018
@andybons
Copy link
Member

andybons commented Mar 7, 2018

/cc @aclements

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Jul 10, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jul 10, 2018
@LDMFD
Copy link

LDMFD commented Sep 28, 2018

Thanks!

@odeke-em
Copy link
Member

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.

@LDMFD
Copy link

LDMFD commented Mar 12, 2019

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

@ianlancetaylor
Copy link
Contributor

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.

@LDMFD
Copy link

LDMFD 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?

@LDMFD
Copy link

LDMFD 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
Copy link
Member

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.

@cuonglm
Copy link
Member

cuonglm 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
Copy link
Contributor

@Gnouc Thanks for the correction.

@LDMFD
Copy link

LDMFD commented Apr 5, 2019

Docs updated in f947c4d to reflect the behaviour of panic()

Thanks all!

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. early-in-cycle A change that should be done early in the 3 month dev cycle. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Status: Triage Backlog
Development

No branches or pull requests

8 participants