Don't defer Close() on writable files (2017)
191–200 of 311 posts
Re: Don't defer Close() on writable files (2017)
#192I’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?
Sometimes there is nothing you can do when there is an error, in that case there is no point in adding several layers of error forwarding until you ignore it somewhere higher up.
Re: Don't defer Close() on writable files (2017)
#193Here is my favorite solution to this problem // CheckClose is a utility function used to check the return from // Close in a defer statement. func CheckClose(c io.Closer, err *error) { cerr := c.Close() if *err == nil { *err = cerr } } Use like this - you must name the error return func whatever() (err error) { f, err := os.Open(blah) // ... defer CheckClose(f, &err) // ... } This closes the file and if there wasn't…
I caution against this approach, as you are not really dealing with the error when it occurs. If the work you do after the defer has other side effects, you may have just gotten your application into an inconsistent state and it's very hard to see in code why this might be. `defer` is really not well-suited for error handling, its benefit is mainly in resource cleanup where failure is impossible or doesn't matter. (T…
Can you give an example case of how this could happen?
Re: Don't defer Close() on writable files (2017)
#194Earlier quoted context omitted.
> It also goes the other way - the update to the directory can be fsynced but the file lost. This can break the "create temp file, write, close, rename to current" scenario (when the intention is to replace file contents atomically). > POSIX doesn't guarantee the order in which data hits the disk, so the above scenario can become "create temp file, write [contents still in memory only], rename to current [written to…
Sqlite is much better than raw files to keep data intact on power failures, and be sure to study the options carefully If you truly need to use files you can take other steps such as mv the old file to .bck before mv the new file, but I really think you want sqlite
Re: Don't defer Close() on writable files (2017)
#195Earlier quoted context omitted.
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.
IMO it's better to think of a directory as "containing" only the named references to inodes. Other directories on the same filesystem may contain other named references to the same inodes ("hard links"). The inodes themselves are more like free floating anonymous objects independent of any particular directory, and might not have a named reference at all (O_TMPFILE; or the named reference was deleted but a file descr…
And I think it makes the collection/container terminology distinction sharper too. Depending on context, I think it’s usually reasonable (if imprecise) to describe a bucket of references or pointers to things as a collection of those things. But I don’t think it makes as much sense to call it a container of those things, except in a really abstract sense.
Re: Don't defer Close() on writable files (2017)
#196Earlier quoted context omitted.
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…
Only if you can safely assume the OS, file system, or std lib cleans up any open file handles that failed to close; I'm 99% sure this is the case in 99% of cases, but there may be edge cases (very specific filesystems or hardware?) where it does matter? I don't know.
Certainly, in the write failure case where there is write failure you'd want to try writing to something else (ideally), notify someone that an operation didn't happen (as a last resort), or something to that effect in order to recover.
But in this case there is no need to try again and nobody really cares. Everything you needed the resources for is already successfully completed. If there is failure when releasing those resources, so what? There is nothing you can do about it.
Re: Don't defer Close() on writable files (2017)
#197Earlier 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…
Re: Don't defer Close() on writable files (2017)
#198Earlier quoted context omitted.
> otherwise the file can be fsynced but the update to the directory lost It also goes the other way - the update to the directory can be fsynced but the file lost. This can break the "create temp file, write, close, rename to current" scenario (when the intention is to replace file contents atomically). POSIX doesn't guarantee the order in which data hits the disk, so the above scenario can become "create temp file,…
> It also goes the other way - the update to the directory can be fsynced but the file lost. This can break the "create temp file, write, close, rename to current" scenario (when the intention is to replace file contents atomically). > POSIX doesn't guarantee the order in which data hits the disk, so the above scenario can become "create temp file, write [contents still in memory only], rename to current [written to…
Of course, the fun part is that the filesystem can’t really guarantee fsync behavior if drives lie about it which many consumer drives do for benchmark reasons. Fun, no?
Re: Don't defer Close() on writable files (2017)
#199Earlier quoted context omitted.
Sqlite is much better than raw files to keep data intact on power failures, and be sure to study the options carefully If you truly need to use files you can take other steps such as mv the old file to .bck before mv the new file, but I really think you want sqlite
SQLite doesn't do any magic other than fsync. Using it to deal with power failures is nonsense.
So while it doesn’t do any other magic, it’s more likely to handle power failures correctly.
Re: Don't defer Close() on writable files (2017)
#200Earlier quoted context omitted.
Go error handling is brutal. I miss exceptions
Exceptions aren't exceptional though; they are too expensive for not-exceptional errors, like failing writes. That said, a language feature where you can throw lightweight error values without generating a stack trace etc might be a middle ground. But it won't land in Go, given the discussion about alternative error handling some years ago. Anyway, in practice it's not that bad. A write can go wrong, you as a develop…
> Exceptions lead a developer to miss errors, or to not handle them in a finegrained manner - e.g. generic "catch all, log error, maybe" style error handling.
I don't see how Go error handling makes people handle things any more explicitly than exceptions. Most people just `if err != nil { return err }`, which to be honest is the _correct_ logic in many cases, and it's pretty easy to forget to check if err and keep on trucking. At least with exceptions if you don't catch it your thread terminates making unhandled exceptions
Exception bubbling means its easier to catch the error at the level that makes sense, and because they are real objects type checking is easier as opposed to the performance of `errors.Is()` which is surprisingly slow.