Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Don't defer Close() on writable files (2017)

joeshaw.org

Post reply on HN
Print view View original

81–90 of 311 posts

Re: Don't defer Close() on writable files (2017)

#81

Post by lostmsu »

neonsunset wrote: post #78
lostmsu wrote: post #77

Earlier quoted context omitted.

I don't think this does the right thing. You are catching exception if Close fails and nothing else. The problem thought is what to do when after file is opened something fails first, and then Close also fails.

I'm catching any exception that occurs within the scope of try block. This includes an exception when trying to open or write to the file. If I fail to write to a file, it will try to dispose the stream, which flushes it and closes the file handle. If disposing the file handle itself fails, which should never happen, the exception will occur in the finally block, which this exception handler catches too. If you need…

Yes, and this ignoring of the original exception is the core of the problem discussed. If you are willing to lose supposedly written data, your approach is golden.

Re: Don't defer Close() on writable files (2017)

#82

Post by everforward »

dist-epoch wrote: post #13
masklinn wrote: post #7

There are more wrinkles with this: - if you are creating a file, to ensure full synchronisation you also need to fsync the parent directory, otherwise the file can be fsynced but the update to the directory lost - if sync fails, you can not assume anything about the file, whether on-disk or in memory , critically one understanding which got dubbed "fsyncgate" and lead to many RDBMS having to be updated is that you ca…

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. For files this is obviously when you write data to them. For directories, this is any time you change the inodes they contain, since you’re effectively writing data to them.

You only need to sync the direct parent because the containers aren’t transitive; the grandparent directory only stores a reference to the parent directory, not to the files within the parent directory. It’s basically a graph.

Re: Don't defer Close() on writable files (2017)

#83

Post by everforward »

saurik wrote: post #39
nickcw wrote: post #23

Here 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…

Which of course isn't really a "solution", as it results in ambiguous error semantics; this is especially the case with Go's defer, as it pushes the code to the end of the entire function, not merely some intermediate relevant scope... the result is that a file used near the beginning of a function might fail to close but that error will not be realized until after something else in the function fails, after the poin…

I think some of those issues with errors could be worked around. Wrapping errors provides a way to return a "cleanup failed" error that also includes the root cause of being a failed write. Likewise, there are packages for handling multi-errors.

I think the reality is that most of us push anything where the return status of Close would be important into the database, specifically because it handles semantics like this and simultaneous writes for us. It's like half the selling point of SQLite; you could write JSON documents and handle all the edge cases yourself, or just jam it in SQLite and quite worrying about Close and simultaneous writes and all that junk.

Re: Don't defer Close() on writable files (2017)

#84

Post by neonsunset »

lostmsu wrote: post #81
neonsunset wrote: post #78

Earlier quoted context omitted.

I'm catching any exception that occurs within the scope of try block. This includes an exception when trying to open or write to the file. If I fail to write to a file, it will try to dispose the stream, which flushes it and closes the file handle. If disposing the file handle itself fails, which should never happen, the exception will occur in the finally block, which this exception handler catches too. If you need…

Yes, and this ignoring of the original exception is the core of the problem discussed. If you are willing to lose supposedly written data, your approach is golden.

As said in the previous comment, you can place a variable in an outside scope and assign to it from within an try-catch block to handle an open file stream in a particular way upon failure. You can simply write more code to disambiguate and specifically handle errors from separate parts of the execution flow. In any case closing a file handle should never fail, but if it does - there are tools to deal with this.

Re: Don't defer Close() on writable files (2017)

#85

Post by adrianmsmith »

thiht wrote: post #68
adrianmsmith wrote: post #4

Surely this would all go away if Go had an exception handling mechanism like most mainstream languages do? You'd just concentrate on the "happy path", you'd close the file, there'd be nothing to forget or write blog posts about because the exception would be propagated, without needing to write any lines of code.

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.

Re: Don't defer Close() on writable files (2017)

#87

Post by saurik »

everforward wrote: post #83
saurik wrote: post #39

Earlier quoted context omitted.

Which of course isn't really a "solution", as it results in ambiguous error semantics; this is especially the case with Go's defer, as it pushes the code to the end of the entire function, not merely some intermediate relevant scope... the result is that a file used near the beginning of a function might fail to close but that error will not be realized until after something else in the function fails, after the poin…

I think some of those issues with errors could be worked around. Wrapping errors provides a way to return a "cleanup failed" error that also includes the root cause of being a failed write. Likewise, there are packages for handling multi-errors. I think the reality is that most of us push anything where the return status of Close would be important into the database, specifically because it handles semantics like thi…

I mean, we can cause the same kind of problem there, as people might try to write a scope exit handler to commit a transaction. You'd then run into the same issue, and so "commit transaction" isn't a thing which should ever be in such a construct. Of course, deleting the objects for the transaction / closing the database connection / etc. would be fine to ignore errors from (and hopefully wouldn't/shouldn't fail anyway) and so those can and should be automated: you defer close but manually call and check a commit.

Once you accept this reality, the file case is the same: putting the sync and/or first critical close inline is equivalent work. The issue is that you simply can't -- no matter what the mechanism is -- slip back and forth between your scope maintenance and your error handling monads, resulting in needing cleanup operations where failure is not an option; and, so, you either must not care about the error in the context of the call or must do something even more drastic like terminate the entire program for violating semantics.

FWIW, I do appreciate that people are less likely to make that kind of mistake when working with a database, as people largely get that you should even try to commit a transaction if the code in it had failed somehow. Additionally, I appreciate that if you have a very tight scope -- which Go makes hard, but can still be pulled off -- the "close and throw an error if and only if we don't have an error right now" strategy is not at all horrible... it just isn't a "solution" to the underlying issue without an understanding of why.

Put elsewise, I think it is useful to appreciate that there is more of a universal theoretical / math reason why this is awkward and why it kind of needs to be built in a specific way, and that this issue transcends the syntax or even the implementation details of how you are trying to manage errors: at the end of the end of the day, all of these techniques people discuss are in some sense equivalent, and, at best, most of these workarounds at offer are ways to incorrectly model the problem due to some systems giving you too much rope.

Re: Don't defer Close() on writable files (2017)

#89

Post by lostmsu »

neonsunset wrote: post #84
lostmsu wrote: post #81

Earlier quoted context omitted.

Yes, and this ignoring of the original exception is the core of the problem discussed. If you are willing to lose supposedly written data, your approach is golden.

As said in the previous comment, you can place a variable in an outside scope and assign to it from within an try-catch block to handle an open file stream in a particular way upon failure. You can simply write more code to disambiguate and specifically handle errors from separate parts of the execution flow. In any case closing a file handle should never fail, but if it does - there are tools to deal with this.

Well the point it: the problem is not as simple in C# as your initial snippet suggests.

Re: Don't defer Close() on writable files (2017)

#90

Post by d0mine »

pansa2 wrote: post #37

AFAIK Python/C# use a similar approach to Go - instead of `defer`, they have `using`/`with` statements. Go's `defer` seems more flexible though - it can execute custom code each time it's used, whereas `using`/`with` always call `__exit__`/`Dispose`. How does the Python/C# approach compare to Go's in this situation? How are errors in `__exit__`/`Dispose` handled?

> Exceptions that occur during execution of this method will replace any exception that occurred in the body of the with statement

https://docs.python.org/3/library/stdtypes.html#contextmanag...

Post reply on HN