-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: Exit does not respect deferred functions making any cleanup code useless #38261
Comments
This is working as intended (and it's well documented, both in the spec section about defer and in the os.Exit documentation); so I don't believe this is a bug.
In Would that satisfy your use case? |
No, because it does not close the entire process just the calling goroutine. Therefore the main goroutine (or other routines which have been spawned) keep running. |
Since |
@ccbrown I already explained the use case. And I think contexts are not applicable because you can't interrupt sleeping goroutines. What about blocking file operations for example. If you look at my example, you will see that Cancelation won't work here as the main routine is not able to cancel the goroutine (the process exit is triggered by the goroutine itself). And I think it's a common task to do some cleanup if the user interrupts the process with SIGINT before exiting. The other way arround I'm question myself why we have os.Exit if it's not applicable to reliable close a process. It does not respect defers. Some of go's core features. |
The way you would do this with contexts looks like this: package main
import (
"context"
"os"
"os/signal"
"time"
"golang.org/x/sync/errgroup"
)
func GoroutineThatSleeps(ctx context.Context) error {
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(time.Second):
return nil
}
}
func main() {
ctx, cancel := context.WithCancel(context.Background())
go func() {
ch := make(chan os.Signal, 1)
signal.Notify(ch, os.Interrupt)
<-ch
cancel()
}()
var g errgroup.Group
// launch some goroutines that we can cancel
for i := 0; i < 3; i++ {
g.Go(func() error {
return GoroutineThatSleeps(ctx)
})
}
if err := g.Wait(); err != nil {
println(err.Error())
os.Exit(1)
}
} https://play.golang.org/p/YAarsVHJSGp This works fine for goroutines that sleep and goroutines that use blocking IO. Many third-party APIs have direct support for using contexts in this way. I said I couldn't think of any good use-cases for a function that attempts to immediately end all goroutines gracefully. There are better, more elegant ways to gracefully cancel goroutines. |
Also want to add: If your concern is with canceling work done by a third-party library that does something like |
Maybe golang does not work like other languages like C if they're calling blocking I/O, but as far as I know blocking I/O is another example for a sleeping thread which cannot be interrupted until a timeout or completion of the operation. At least my proposal whould allow developers to recover if for example some goroutine panics. Currently only the panicking goroutine is able to do something in case of an error. If some other routine hasn't completed it's operation this could cause data corruption, just for example.
I just wanted to show that without support by os.Exit() or another API function it forces you to use many more lines of code if you compare this to just an simple exit call. |
Yeah this is just a misunderstanding. You can cancel goroutines that are already sleeping or doing blocking I/O. My example function sleeps for one second. If you interrupt the process half a second into the goroutine's slumber, it'll be canceled and return immediately. Try making the sleep time one minute and interrupt it a few seconds in and this will be easier to see: You won't have to wait the full minute. |
I know this is not the right place to ask questions, but in the example the magic is, that the |
Unfortunately func ReadAllWithContext(ctx context.Context, path string) (bufOut []byte, errOut error) {
f, err := os.Open(path)
if err != nil {
return nil, err
}
defer f.Close()
// Spawn a goroutine to close the file if the context is canceled.
done := make(chan struct{})
go func() {
select {
case <-ctx.Done():
f.Close()
case <-done:
}
}()
defer close(done)
// If the context was canceled before we could read the file, change the error to ctx.Err().
defer func() {
if ctxErr := ctx.Err(); errOut != nil && ctxErr != nil {
errOut = ctxErr
}
}()
return ioutil.ReadAll(f)
}
|
Ordinary Go code doesn't have blocking I/O operations in the sense that I think you mean. When a goroutine calls But I'm not sure what that has to do with the behavior of In general there is no reliable way to do anything as a program exits, because there are many different ways that a program can exit. For example, on GNU/Linux, the kernel can kill it if it starts using too much memory. Therefore, if there is any cleanup that must be done on program exit, the only reliable way to do it is to invoke the program from a different process that just does the cleanup. The Go standard library has chosen to not provide a 90% solution in this space, because it is a space where correct functioning often requires a 100% solution. |
@Sebi2020 if you can get data corruption due to unexpected exit, that will happen if the host crashes or powers off. You should be able to recover on restart, and not expect graceful exit. If you're using a transactional data store, this isn't hard to do. If using the filesystem, it's much more involved (speaking from experience). |
@networkimprov I think that are two different problems yielding to the same result. But with the difference that a power loss or crash off the host is out of your control (from the viewpoint of the application developer), if you have not taken any steps to mitigate these cases. But how an application exits is within your control (mostly, not always). As I said I just think that a language construct like |
It sounds like you just want to fire and forget goroutines, but still be able to stop them gracefully. That’s really not something that’s likely to ever be supported though, and this behavior isn’t just specific to Go. In other languages that support RAII patterns like Rust or C++, destructors don’t execute if you |
@ccbrown most of the other languages have other methods ensuring that cleanup code can run on a call to exit. This doesn't necessary have to happen inside a destructor (or a defer call) but it would make things much less annoying. An example in C++ is |
I think you misunderstand my point a bit. Go's I wasn't necessarily suggesting that Go shouldn't have an |
Seems like this is working as documented. If you have a specific proposal in mind, please file a new issue with details. For now, I'm closing this issue as there doesn't seem to be anything else to do here. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I called os.Exit(1) on SIGINT
https://play.golang.org/p/_Py0WToAf8v
(Local testing is better, playground does not like sleeps and have no way to cause a SIGINT).
What did you expect to see?
Golang runs all deferred cleanup routines. This would allow every go routine to register it's own cleanup function if you think separation of concerns is a thing.
What did you see instead?
No Cleanup function is called at all, but that is important if you're writing to files in other goroutines. Panic have a similiar issue.
See also: #4101
The text was updated successfully, but these errors were encountered: