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

111–120 of 311 posts

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

#111

Post by holowoodman »

RenThraysk wrote: post #105
everforward wrote: post #82

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

In the case of atomic file writing, the rename() doesn't cause a fsync of the parent directory?

Depends on the file system. In NFS, yes. In others, maybe not.

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

#112

Post by anon-3988 »

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?

Is anything ever enough aside from powering down the PC and then reading it back again to make sure its there?

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

#113

Post by praptak »

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…

> 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, write [contents still in memory only], rename to current [written to disk], power failure".

I believe there was a bug where this scenario had worked for a long time then one filesystem (ext4?) pushed closer to what is admissible under POSIX (non-obvious reorders of physical writes) and people started getting random data corruption in programs which used write & rename.

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

#114

Post by altfredd »

RenThraysk wrote: post #105
everforward wrote: post #82

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

In the case of atomic file writing, the rename() doesn't cause a fsync of the parent directory?

> rename() doesn't cause a fsync of the parent directory?

It does not.

At best it will schedule a journal commit asynchronously (I recall that ext4 maintainer complained about adding this "workaround for buggy user code" on lkml). If you want to receive an IO error when renaming fails, make sure to call fsync() yourself.

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

#116

Post by weinzierl »

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…

"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."

And if you need this in Java you still have resort to ugly hacks.

https://github.com/apache/lucene/issues/7231

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

#118

Post by foldr »

bryancoxwell wrote: post #107
randomdata wrote: post #93

Earlier quoted context omitted.

> I still question why defer doesn't support doing exactly that. When would it ever be useful? You'd soon start to hate life if you actually tried using the above function in anything beyond a toy application. > 99% of the time shouldn't be used 1. 99% of the time it is fine to use without further consideration. Even if there are errors, they don't matter. The example from the parent comment is a perfect case in poin…

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 cases, the solution to this problem will be to use a WriteFile function that opens, writes and closes the file and does all the error handling for you.

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

#119

Post by kokada »

layer8 wrote: post #55
cyco130 wrote: post #26

Earlier quoted context omitted.

This problem isn't solved with exceptions either. The problem is that finalizers (C++ destructors, Java's `finally` blocks, Go's `defer` etc.) shouldn't fail but `close()` can fail. Therefore, for 100% correctness, `close()` calls should be handled explicitly and not left to finalizers. Finalizers shouldn't fail because they might be executed while another exception is already in flight. Three languages have three di…

Java with try-with-resources does the correct thing: It attaches the new exception as a secondary exception to the currently in-flight exception. Since function calls form a tree, exceptions must form a tree as well. Doing this automatically is also one of the killer arguments for exceptions over error codes, IMO.

Go also has `errors.Join` since 1.20, and one of its uses is exactly this one: to be used in deferred close that can possible raise errors so the errors can stack each other: https://stackoverflow.com/a/78013962.

> Doing this automatically is also one of the killer arguments for exceptions over error codes, IMO.

Definitely doing this automatically is better than relying on the programmer to do this manually, but I wouldn't say this is the "killer" argument for exceptions over errors codes, because this doesn't add anything new to the argument of exceptions vs explicit error handling.

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

#120

Post by fragmede »

im3w1l wrote: post #117
im3w1l wrote: post #102

Didn't I see this thread the other day including comments? Investigating, Algolia search shows this thread as being posted 2 days ago, and the memorable comments too.

@dang did the timestamps get messed up?

https://news.ycombinator.com/item?id=26998308
Post reply on HN