`File::close`

[tbu-]

Proposal

Problem statement

Some projects want to observe the return value of the close(2) syscall on Linux, when closing files. The standard library currently does not expose this functionality, because close(2) is called in the destructor of File which does not have the ability to communicate an error.

Motivating examples or use cases

E.g. pola-rs/polars#21002 where a user of the polars Python library would like to get informed about close(2) errors because they indicate problems with closing files over NFS in their setup. There's a workaround of manually calling close(3) from libc: pola-rs/polars#21588.

Solution sketch

impl File {
    /// Close a file, returning any reported errors from the OS.
    ///
    /// If you want to make sure the data actually hit the disk, use [`File::sync_data`] instead.
    fn close(self) -> io::Result<()>;
}

impl OwnedFd {
    fn close(self) -> io::Result<()>;
}

Alternatives

This can be done in a third-party crate.

Links and related work

• Current handling of Unix close() can lead to silent data loss rust#98338
• Prior art in other programming languages and other software: File::close #705 (comment).

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

• We think this problem seems worth solving, and the standard library might be the right place to solve it.
• We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

• We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
• We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

[FeldrinH]

If this goes ahead then I think it is worth considering if close should be exposed just on concrete types like File or maybe more generally through some kind of trait (either a separate trait or a subtrait/new method on Write).

There are types like BufWriter where it would be convenient to have BufWriter::close, which flushes the buffer, closes the underlying writer, and returns any errors encountered.

[the8472]

would like to get informed about close(2) errors because they indicate problems with closing files over NFS in their setup

They're asking for non-portable behavior, something that only does error-reporting on NFS (and this is not even a general property of NFS, it's the linux client's cto mount option).
That doesn't mean std should encourage such non-portable uses, because errors will remain unreported on most other filesystems due to writeback.

All that close guarantees is decrementing the reference count of the underlying resource by one. That's it. Neither error reporting nor any IO happening are guaranteed. In fact new IO can continue to happen even after an FD has been closed (due to mmap).

Half-serious: File::decrement_ref_count(self) -> Result<()>

@dtolnay you previously recommended writing an RFC, is that still your position?

[tbu-]

They're asking for non-portable behavior, something that only does error-reporting on NFS (and this is not even a general property of NFS, it's the linux client's cto mount option).

Yes, they're asking for nonportable behavior that works in their setup. In this case, close returns a useful error to them, so they'd like to be able to see that error.

Errors will remain unreported in other cases as you've pointed out, they're just interested in this particular problem they hit, where they can't see the error on close(2) on their Linux NFS setup "with cto mount option".

All that close guarantees is decrementing the reference count of the underlying resource by one. That's it. Neither error reporting nor any IO happening are guaranteed. In fact new IO can continue to happen even after an FD has been closed (due to mmap).

This ACP does not talk about guarantees except in the recommendation to use File::sync_data if you want some.

[the8472]

If you want system-specific behavior you already can get that safely with platform-specific crates like nix, the standard library isn't needed to provide that.
Is there a portable use-case for a portable close API? I'm skeptical that there is any, due to the dearth of guarantees.

[ChrisDenton]

The portable use case would be for logging the error. This may potentially help with debugging problems.

[FeldrinH]

Is there a portable use-case for a portable close API? I'm skeptical that there is any, due to the dearth of guarantees.

For me a big part of portability is trying to "play nice" with as many implementations as possible. Some filesystems use close to report write errors, and accomodating them is in my mind a part of writing portable software.

[the8472]

For me a big part of portability is trying to "play nice" with as many implementations as possible

sync_all is the superset that works with all standards-compliant filesystems. that's the portable approach. It seems contorted to me to call the thing that only achieves the desired behavior (data not getting lost) on specific platforms, filesystems and mountoptions as being portable.

Some filesystems use close to report write errors,

Even in the specific case of linux nfs with cto that still isn't guaranteed. My understanding is that it does the flushing only when the file description gets released, not every file descriptor.
So if there was process spawning going on then a copy of the file descriptor temporarily lives in the forked child. When the parent then closes the file it doesn't see any errors because the reference count is not zero, only when the child execs does it get closed and the errors just get cast into the void.

[FeldrinH]

sync_all is the superset that works with all standards-compliant filesystems. that's the portable approach.

Point taken. I just think there is a sizable subset of programs that will never use fsync or equivalent because of the performance implication, but would still like to make a best-effort attempt to detect failed writes (admittedly this belief is mostly based on my own experience, not any statistical data).

Some examples of such programs that I can think of are polars (mentioned above) and coreutils cp. These programs will probably never use fsync by default because they would be swamped by people complaining about bad file writing performance, but they still care about failed writes and want to make an effort to detect and report them.

[FeldrinH]

Overall, I feel like this discussion is going around in circles. Ultimately there is no optimal solution for all cases, so this is all about balancing tradeoffs and we just have different views on what tradeoffs should be made.

[the8472]

but they still care about failed writes and want to make an effort to detect and report them.

Then they should try RWF_DONTCACHE or sync_file_range to at least initiate writeback even if not blocking on its completion. cp copying multiple files could also keep a pool of file descriptors that still are waiting on writeback and get checked later rather than immediately closing them, that way they'd also increase the chance of seeing errors.

It's a really weird position to say they care about data being written but then not actually doing anything to make it more likely to happen.

[interacsion]

For what it's worth, closing a file on Windows is also seemingly fallible: https://learn.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-closehandle

[max-heller]

What about exposing just a low-level function like std::os::fd::close(fd: RawFd) -> io::Result<()> to avoid presenting too pretty of an API and allow for e.g. retrying on EINTR? Closing a file would look like fd::close(file.into_raw_fd()), which isn't as clean as file.close(), but maybe that's a good thing? This can of course already be written by pulling in libc/nix/etc., but I wonder if having the primitive in std would satisfy some of the desire for a built-in method.