Java's Closeable.close() has declared multiple invocations as safe for 20 years, when it first became an interface.
Don't defer Close() on writable files (2017)
241–250 of 311 posts
Re: Don't defer Close() on writable files (2017)
#242Earlier quoted context omitted.
The whole point of this post is that an error returned from file.Close DOES matter
I think from a Go point of view, the lesson to be drawn from that is "don't defer a function call if you need to check its error value", rather than "defer needs to support checking of function return values". In the example at hand, it really makes more sense to call Close() as soon as possible after the file is written. It's more of an issue with the underlying OS file API making error checking difficult. In 99% of…
> the lesson to be drawn from that is "don't defer a function call if you need to check its error value"
Isn't the lesson here: If you must have a Close method that might fail in your API, ensure it can safely be called multiple times?
As long as that is true, you can approach it like you would any other API that has resources that might need to be cleaned up.
f, _ := os.Create(...)
defer f.Close()
// perform writes and whatever else
if err := f.Close(); err != nil {
// recover from failure
}
> the solution to this problem will be to use a WriteFile function
If it were the solution you'd already be using os.WriteFile. It has a time and place, but often it is not suitable. Notably because it requires the entire file contents to be first stored in memory, which can become problematic.
Certainly you could write a custom WriteFile function that is tuned to your specific requirements, but now you're back to needing to be familiar with the intricacies of a lower-level API in order to facilitate that.
Re: Don't defer Close() on writable files (2017)
#243Earlier quoted context omitted.
> It’s basically a graph. do you mean it's a basically a tree? Because if it were just a graph, you could still have edges from the grandparent to the grandchild in addition to the one from the former to the child
This ... depends. In normal POSIX land, hard links break the tree structure, for one, so you get a DAG but not a tree. I think some file systems do enforce tree structure, though - hard links are not supported everywhere. It used to be possible ages ago to hard link to directories, which meant that you could have actual cycles and a recursive tree-walking algorithm would never terminated. (As far as I know you can st…
> , hard links break the tree structure, for one, so you get a DAG but not a tree.
More than DAGs, but instead pretty arbitrary graphs since you can express cycles with hard links.
Re: Don't defer Close() on writable files (2017)
#244Earlier quoted context omitted.
I think from a Go point of view, the lesson to be drawn from that is "don't defer a function call if you need to check its error value", rather than "defer needs to support checking of function return values". In the example at hand, it really makes more sense to call Close() as soon as possible after the file is written. It's more of an issue with the underlying OS file API making error checking difficult. In 99% of…
> the lesson to be drawn from that is "don't defer a function call if you need to check its error value" Isn't the lesson here: If you must have a Close method that might fail in your API, ensure it can safely be called multiple times? As long as that is true, you can approach it like you would any other API that has resources that might need to be cleaned up. f, _ := os.Create(...) defer f.Close() // perform writes…
Sure, that's an alternative, although it means there will be some code paths where the error returned by f.Close() becomes the error returned by the entire function and others where it is ignored (though you could easily log it). That might be fine, but you also might want to handle all the cases explicitly and return a combined error in a case where, say, a non-file-related operation fails and then the file also fails to close.
Re: Don't defer Close() on writable files (2017)
#245I defer these AND check the errors. https://pkg.go.dev/go.uber.org/multierr#hdr-Deferred_Functio... has a nice API.
I wrote my own version for $WORK package (as our policy is that all errors must be wrapped with a message), used like:
func foo() (retErr error) {
x := thing.Open(name)
defer errors.Close(&retErr, x, "close thing %v", name)
...
}
Finally, errcheck can detect errors-ignored-on-defer now, though not when run with golangci-lint for whatever reason. I switched to nogo a while ago and it found all the missed closes in our codebase, which were straightforward to fix. (Yes, I wish the linter auto-ignored cases where files opened for read had their errors ignored. It doesn't, so we just check them.)
Multi-errors are nice.
Re: Don't defer Close() on writable files (2017)
#246Earlier quoted context omitted.
The whole point of this post is that an error returned from file.Close DOES matter
You seem confused. The article is about writing a file where it does matter, but the comment example, which is what we're talking about, only reads a file. If close fails after read, who gives a shit? What difference is it going to make? All your read operations are complete already. Close isn't going to trigger a time machine that goes back and time and undos the reads you've performed. It is entirely inconsequentia…
> If close fails after read, who gives a shit?
ulimit -n
You ignore errors on close, and one morning you wake up with your app in CrashLoopBackoff with the final log message "too many files". How do you start debugging this?
Compare the process to the case where you do log errors, and your log is full of "close /mnt/some-terrible-fuse-filesystem/scratch.txt: input/output error". Still baffling of course, but you have some idea where to go next.
Re: Don't defer Close() on writable files (2017)
#247I’m a bit new to Golang, but not good programming practices. Isn’t ignoring the error value returned by a function a very bad practice in general? Regardless of if the function call returning it is used in defer? Not just for file write operations?
Ignoring errors is generally a poor practice. You don't have to stop your program on errors, but you should at least log some percentage of them so that when the failure cascades, you have some idea where the failure started.
Re: Don't defer Close() on writable files (2017)
#248Earlier quoted context omitted.
Please no. Just handle errors from `close()` when you're writing files. > On modern hardware if a disk is throwing an io error on write you are having a bad day. And how would you know you're having a bad day if apps ignore those errors?
I'm arguing that the proper thing to do here is to kill the process along with whatever else is using the block device. Whether you handle the error or immediately or if you allow the error to occur after a defer, you still are almost certainly not handling it properly and are taking a speed hit for your troubles.
Using signals for this really sucks. Anyways, you don't get to do this now because of backwards-compatibility. Just handle the error.
Re: Don't defer Close() on writable files (2017)
#249Never thought that `Close` could fail, as `CloseHandle` on Windows never fails. I wonder how .NET behaves in the same scenario on Linux.
Re: Don't defer Close() on writable files (2017)
#250Boggles my mind that after more than 60 years of computer science, we still design tools (programming languages) where the simplest tasks are full of gotchas and footguns. This is a great example.
Windows doesn't have this gotcha, FWIW.