-
Notifications
You must be signed in to change notification settings - Fork 100
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
Wrap main and remove os.Exit calls so all deferred functions will execute #1693
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's the right shape, but I wonder what we can do with these errors.
main
doesn't have a logger, and stdout may not go anywhere. I wonder about a roundabout thing where runMain
returns a slogger, and and error, and if the slogger isn't nil it can log the error... Or we can make sure all the error returns log first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't test this, but it looks okay
fc6e4fc
to
e549436
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm game. I don't think I was that confident when I wrote service manager stuff, so this feels like a reasonable upgrade.
Relates to #1692
When we call os.Exit, deferred functions will not execute, which means e.g. the event log writer on windows won't get closed. For a more graceful shutdown, we wrap main as
runMain
and disallowos.Exit
calls outside of the newmain
.