Can someone tell me what’s going on here? This very post including the comments appeared on HN two days as well. I thought I was getting crazy but Google confirms.
Don't defer Close() on writable files (2017)
131–140 of 311 posts
Re: Don't defer Close() on writable files (2017)
#132Earlier quoted context omitted.
Exceptions are a terrible error handling mechanism. You have no idea what throws and what doesn’t, it’s impossible to write defensive code that makes sense with exceptions. Errors as value is the only sane way to deal with errors. Granted, Go does it pretty badly but it’s still infinitely better than exceptions.
> You have no idea what throws and what doesn’t The answer here is that everything throws. Any code can have a Null/Nil dereference error, any code can use an array and generate an out-of-bounds exception, etc.
When there's a null pointer dereference, that's a bug in the program; when closing a file fails, that's something external the program must handle.
Re: Don't defer Close() on writable files (2017)
#133Earlier quoted context omitted.
So if I do fopen/fwrite/fsync/fclose, that is not enough? That is crazy, I think 90% of apps don't fsync the parent directory. Also, how many levels of parents do you need to fsync?
That should be only for creating files, and maybe updating their metadata (not sure about that one). The confusion stems from people thinking that files and directories are more different than they are. Both are inodes, and both are basically containers for data. File inodes are containers for actual data, while directory inodes are containers for other inodes. All inodes need to be fsynced when you write to them. Fo…
> while directory inodes are containers for other inodes.
Uh? No.
You can imagine them as a list of name,inode tuples.
Re: Don't defer Close() on writable files (2017)
#134Earlier quoted context omitted.
I am talking about posix semantics yes, I have no idea how things work on windows.
Phew. I've primarily used Windows. Not that any of the posix programs I've been exposed to have done the dir sync though. For cross-platform stuff I've mainly used Boost, which I assumed handled such details.
I'm sure it does not.
Also these things are needed very very rarely (which is why few even know about the issue) and are not good for performance and battery life.
Re: Don't defer Close() on writable files (2017)
#135The snippet in the first update is very wrong. The manpage for Linux’s implementation of close() explicitly says that it should not be called again if it fails. Apparently, this is the same under FreeBSD.
The implementation of `os.File.Close()` in Go clears the file descriptor.
https://pkg.go.dev/os#File.Close
“Close will return an error if it has already been called.”
An os.File is a data structure containing a file descriptor and some other fields. It is safe to call Close() multiple times, because it will only call the underlying syscall close() once.
Re: Don't defer Close() on writable files (2017)
#136TL;DR - defer Close ignores the error from Close, so don't.
Eh, the real answer is close twice (as in the article update):
func f(filename string) error {
fp, err := os.Create(filename)
if err != nil {
return err
}
defer fp.Close()
if _, err := fmt.Fprintln(fp, "Hello, world!"); err != nil {
return err
}
return fp.Close() // safe to call multiple times
}
Re: Don't defer Close() on writable files (2017)
#137The end of the article has my favorite solution.
This is also how I'd solve it in Rust, except the defer would be implicit.
func doSomething() error {
f, err := os.Create("foo")
if err != nil { return err }
defer func(){ _ = f.Close() }()
if _, err := f.Write([]byte("bar"); err != nil { return err }
return f.Close()
}Re: Don't defer Close() on writable files (2017)
#138Isn’t what the article suggests (with defer and then WriteString) technically a race condition? Is there no way that the closer can get called before WriteString executes?
Re: Don't defer Close() on writable files (2017)
#139The end of the article has my favorite solution. This is also how I'd solve it in Rust, except the defer would be implicit. func doSomething() error { f, err := os.Create("foo") if err != nil { return err } defer func(){ _ = f.Close() }() if _, err := f.Write([]byte("bar"); err != nil { return err } return f.Close() }
Yes, note you’ll also see:
defer f.Close()
defer func(){ _ = f.Close() }()
Re: Don't defer Close() on writable files (2017)
#140Earlier quoted context omitted.
That should be only for creating files, and maybe updating their metadata (not sure about that one). The confusion stems from people thinking that files and directories are more different than they are. Both are inodes, and both are basically containers for data. File inodes are containers for actual data, while directory inodes are containers for other inodes. All inodes need to be fsynced when you write to them. Fo…
> while directory inodes are containers for other inodes. Uh? No. You can imagine them as a list of name,inode tuples.
I’m not well versed enough on the subject to know if there’s important nuance here that I’m missing… but that doesn’t sound like a contradiction to me. That’s effectively a map with O(n) access, right? I think I would more generally refer to that as a “collection”, but it certainly contains inodes as you describe it.