Live data from Hacker News

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

joeshaw.org

171–180 of 311 posts

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

#173

Earlier quoted context omitted.

I've never heard about this in my years of programming. I just tried to read through the Win32 documentation, as I've done several times over the years, and it mentions a lot of edge cases but not this that I could see. Is this some Linux/Unix specific thing? Am I blind?

As far as I know, it's specific to the combination of certain POSIX-ish OS and file systems, like linux/Ext3. I have no clue what BSD does here, or whether ReiserFS is different. Windows/NTFS is a different world, there are still edge cases that can go wrong but I don't think this particular one is a problem because FAT/NTFS is not inode-based. I imagine if you looked at the SQLite source code you'd see different edg…

Ya NTFS file naming and long path is a wreck. Unzipping files from mac/linux is a easy way to end up with missing data. Applications quite often break on long file paths especially Microsofts own stuff like powershell.

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

#174

Earlier quoted context omitted.

I am talking about posix semantics yes, I have no idea how things work on windows.

There's the old saying that on Windows*, files have names; in POSIX, names have files. I think that's what makes the difference here. * technically it's the filesystem as much as the OS that is relevant here.

* a very limited set of names in relation to what's possible on other operating systems.

If your processing files from other systems on NTFS you'll very likely have rename said files in an application and store an index of the names.

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

#175
post #172

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

Funny thing is that there is a near footgun with this go: if you defer and set a non named return in a defer, like cErr, that won’t actually set that variable. Not sure what actually happens in that case but godbolt would tell you. In that case, the error would get swallowed

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

#176
post #133

Earlier quoted context omitted.

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

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 descriptor is still open; or orphaned inodes due to filesystem corruption or someone forgetting to fsync the directory after creating a file as masklinn pointed out - e2fsck will link these in /lost+found). This is also why chmod appears to affect other hard links in different directories: Because it actually modifies the inode (file permissions are recorded in that), not the named reference.

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

#177

I’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)

#178
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?

Great another case of simple thing everyone knows is simple but turns out to be horrifyingly complicated and heavily system dependent, but only on occasion so you can get 80% of the way through your career before encountering the gaps in your knowledge.

I guess I'll add it to the list.

Of course on the other hand, I was already thinking that I should just use SQLite for all my file handling needs. This little nugget makes me think that this was the correct intuition. [Queue horrifying revelations w.r.t. SQLite here.]

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

#179

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

Crazy is the right term. File system APIs in general have too many sharp edges and need a ground-up rethink. Consider S3-like protocols: these recognise that 99% of the time applications just want “create file with given contents” or “read back what they’ve previously written.” The edge cases should be off the beaten path, not in your way tripping you up to when you want the simple scenario.

Aren't the edge cases features? An abstraction (or different API, sure) is in order to prevent footguns. However, this abstraction should not force fsyncs for example, due to the performance impact mentioned. It puts the choice of guaranteed writes vs performance to the developer.

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

#180
post #149

The article suggests using a named return value `err` to allow the return value of `Close` to be be propagated - unless doing so would overwrite an earlier error: defer func() { cerr := f.Close() if err == nil { err = cerr } }() Wouldn't it be better to use `errors.Join` in this scenario? Then if both `err` and `cerr` are non-nil, the function will return both errors (and if both are `nil`, it will return `nil`): def…

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 developer should write code that handles that situation. 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.

Post reply on HN