`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. https://github.com/pola-rs/polars/issues/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: https://github.com/pola-rs/polars/pull/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

• https://github.com/rust-lang/rust/issues/98338
• Prior art in other programming languages and other software: https://github.com/rust-lang/libs-team/issues/705#issuecomment-3591003525.

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.

[hanna-kruppe]

and allow for e.g. retrying on EINTR

Retrying close() on errors is wrong:

Retrying the close() after a failure return is the wrong thing to do, since this may cause a reused file descriptor from another thread to be closed. This can occur because the Linux kernel always releases the file descriptor early in the close operation, freeing it for reuse; the steps that may return an error, such as flushing data to the filesystem or device, occur only later in the close operation.

Many other implementations similarly always close the file descriptor (except in the case of EBADF, meaning that the file descriptor was invalid) even if they subsequently report an error on return from close().

Aside: fn close(fd: RawFd) is no good w.r.t. I/O safety -- the function you're proposing would have to be unsafe just like libc::close. I believe that taking OwnedFd would fix that problem, and also prevent the mistake of retrying on error.

[ChrisDenton]

Yes. The signature you want is to consume an OwnedFd and return a Result<(), io::Error>. Or considering any error at that point is advisory it could be an Option<io::Error>. Though admittedly that looks a bit weird, e.g.:

fn close(fd: OwnedFd) -> Option<io::Error>

[max-heller]

and allow for e.g. retrying on EINTR

Retrying close() on errors is wrong:

I was going based off of OwnedFd::drop():

// Note that errors are ignored when closing a file descriptor. According to POSIX 2024,
// we can and indeed should retry `close` on `EINTR`
// (https://pubs.opengroup.org/onlinepubs/9799919799.2024edition/functions/close.html),
// but it is not clear yet how well widely-used implementations are conforming with this
// mandate since older versions of POSIX left the state of the FD after an `EINTR`
// unspecified. Ignoring errors is "fine" because some of the major Unices (in
// particular, Linux) do make sure to always close the FD, even when `close()` is
// interrupted, and the scenario is rare to begin with. If we retried on a
// not-POSIX-compliant implementation, the consequences could be really bad since we may
// close the wrong FD. Helpful link to an epic discussion by POSIX workgroup that led to
// the latest POSIX wording: http://austingroupbugs.net/view.php?id=529

Aside: fn close(fd: RawFd) is no good w.r.t. I/O safety -- the function you're proposing would have to be unsafe just like libc::close. I believe that taking OwnedFd would fix that problem, and also prevent the mistake of retrying on error.

Ah yes, I'd forgotten that borrowing a RawFd from an OwnedFd is of course safe and shouldn't make it possible through safe code to invalidate the OwnedFd. We'd need something like close(fd: OwnedFd) -> Result<(), (io::Error, Option<OwnedFd>)> if a caller wanted to implement the retry described in the Drop impl.

[the8472]

Someone who needs that function and has carefully studied the guarantees of the particular platform they're targeting and understands IO-safety can use libc or nix. Anyone else has no chance of using it correctly. Afaik none of rust's official targets supports retries. I see even less of a case for having this in std than the one proposed in OP.

[FeldrinH]

Agreed, I don't think there's a legitimate use for retry support. AFAIK after close returns the file descriptor is either closed or in an unspecified state. In either case, retrying close would be unsafe. From https://pubs.opengroup.org/onlinepubs/009604499/functions/close.html (emphasis mine):

If close() is interrupted by a signal that is to be caught, it shall return -1 with errno set to [EINTR] and the state of fildes is unspecified.

Maybe there is some platform where retrying close is safe, but that is so niche that I couldn't name a single example, and at that point using a platform-specific library seems appropriate.

[the8472]

That's POSIX 2004. The 2024 edition has revised this.

Maybe there is some platform where retrying close is safe

HP-UX. Not on the list of supported targets. https://nixdoc.net/man-pages/HP-UX/man2/close.2.html

[scottmcm]

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.

cc "Add the Close trait" https://github.com/rust-lang/rfcs/pull/2677

[tbu-]

From the linked thread (https://github.com/pola-rs/polars/issues/21002):

NFSv4.2. Observed against both Vast and Isilon (Dell) server side, when under load. (And with low occurrence rate, but at scale)

Hoisting a quote from internal discussion

we definitely lose data due to failed close calls. It's also handleable/exposeable at zero cost. Adding fsync does come at a cost, and I haven't seen data loss due to not calling it. It is needed by spec, I am sure it is going wrong in places, but the close bit accounts for >95% of current problems.

I do understand a safety conscious systems language taking the position it does. Polars as data-science library with diverse user base I am very glad made a fix and it did sort our problems.

Being able to observe the return value of close(2) comes at zero runtime cost (we'd call close(2) anyway) and is useful to some use cases. It has a use here, in this setup. It apparently works better in their setup than calling fsync(2), due to lower runtime cost). Thus, I'd say it's useful for the Rust standard library to add a function that is able to observe the return value of close(2).

[tbu-]

Prior art:

Programming languages

C (has close, returns error code)

Code:

#include <stdio.h>

int main() {
	FILE *file = fopen("/tmp/mnt/badfile", "w");
	if(fclose(file)) {
		perror("fclose");
	}
	return 0;
}

Output:

fclose: Input/output error

C++ (has close, function to get error)

Code:

#include <fstream>
#include <iostream>
int main() {
        std::ofstream file("/tmp/mnt/badfile");
        file.close();
        std::cout << file.fail() << "\n";
}

Output:

1

Go (has close, returns error)

Code:

package main
import (
	"fmt"
	"os"
)

func main() {
	err := os.WriteFile("/tmp/mnt/badfile", []byte(""), 0644)
	if err != nil {
		fmt.Printf("WriteFile: %s\n", err);
	}
	file, _ := os.Create("/tmp/mnt/badfile")
	err = file.Close()
	if err != nil {
		fmt.Printf("Close: %s\n", err);
	}
}

Output:

WriteFile: close /tmp/mnt/badfile: input/output error
Close: close /tmp/mnt/badfile: input/output error

Haskell (has close, returns error)

Code:

import System.IO
main = writeFile "/tmp/mnt/badfile" ""

Output:

a: /tmp/mnt/badfile: withFile: hardware fault (Input/output error

Java (has close, throws error)

Code:

import java.io.FileWriter;
import java.io.IOException;

void main() throws IOException {
    var writer = new FileWriter("/tmp/mnt/badfile");
    writer.close();
}

Output:

Exception in thread "main" java.io.IOException: Input/output error
    at java.base/java.io.FileDescriptor.close0(Native Method)
    at java.base/java.io.FileDescriptor.close(FileDescriptor.java:304)
    at java.base/java.io.FileOutputStream$1.close(FileOutputStream.java:399)
    at java.base/java.io.FileDescriptor.closeAll(FileDescriptor.java:361)
    at java.base/java.io.FileOutputStream.close(FileOutputStream.java:397)
    at java.base/sun.nio.cs.StreamEncoder.implClose(StreamEncoder.java:342)
    at java.base/sun.nio.cs.StreamEncoder.close(StreamEncoder.java:159)
    at java.base/java.io.OutputStreamWriter.close(OutputStreamWriter.java:253)
    at a.main(a.java:6)

NodeJS (has close, throws error)

Code:

import { open } from 'node:fs/promises';

let filehandle = await open('/tmp/mnt/badfile', 'w');
await filehandle.close();

Output:

node:internal/modules/run_main:107
    triggerUncaughtException(
    ^

[Error: EIO: i/o error, close] {
  errno: -5,
  code: 'EIO',
  syscall: 'close'
}

Perl (has close, returns error code)

Code:

open(file, '>', '/tmp/mnt/badfile') or die $!;
close(file) or die $!;

Output:

Input/output error at a.perl line 2.

PHP (has close, does not propagate EIO)

Code:

<?php

var_dump(file_put_contents("/tmp/mnt/badfile", ""));

$file = fopen("/tmp/mnt/badfile", "w");
var_dump(fclose($file));

Output:

int(0)
bool(true)

Python (has close, throws error)

Code:

with open("/tmp/mnt/badfile", "w") as f:
    pass

Output:

Traceback (most recent call last):
  File "a.py", line 1, in <module>
    with open("/tmp/mnt/badfile", "w") as f:
         ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^

Ruby (has close, throws error)

Code:

File.open('/tmp/mnt/badfile', 'w') { }

Output:

a.rb:1:in 'IO#close': Input/output error @ fptr_finalize_flush - /tmp/mnt/badfile (Errno::EIO)
	from a.rb:1:in 'IO.open'
	from a.rb:1:in '<main>'

Swift (has close, throws error)

Code:

import Foundation

let file = FileHandle.init(forWritingAtPath: "/tmp/mnt/badfile")!;
try file.close()

Output:

Swift/ErrorType.swift:254: Fatal error: Error raised at top level: Error Domain=NSCocoaErrorDomain Code=256 "(null)"UserInfo={NSUnderlyingError=Error Domain=NSPOSIXErrorDomain Code=5 "Input/output error"}

*** Signal 4: Backtracing from 0x7f2fde1d26e8... done ***

*** Program crashed: Illegal instruction at 0x00007f2fde1d26e8 ***

Platform: x86_64 Linux (Arch Linux)

Thread 0 "a" crashed:

  0      0x00007f2fde1d26e8 _assertionFailure(_:_:file:line:flags:) + 264 in libswiftCore.so
  1 [ra] 0x00007f2fde21dba1 swift_errorInMain + 768 in libswiftCore.so
  2 [ra] 0x00005575320aae2b main + 234 in a
  3 [ra] 0x00007f2fdc93b635 <unknown> in libc.so.6
  4 [ra] 0x00007f2fdc93b6e9 <unknown> in libc.so.6
...


Registers:

rax 0x0000000200000003  8589934595
rdx 0x0000000000000002  2
rcx 0xfffffffe00000000  18446744065119617024
rbx 0x0000000000000003  3
rsi 0x000055756b8898a8  03 00 00 00 00 00 00 00 b0 00 00 00 00 00 00 80  ········°·······
rdi 0x0000000000000006  6
rbp 0x00007ffc068fa510  a0 a5 8f 06 fc 7f 00 00 a1 db 21 de 2f 7f 00 00   ¥··ü···¡Û!Þ/···
rsp 0x00007ffc068fa4a0  b0 a4 8f 06 fc 7f 00 00 bb 95 0f de 2f 7f 00 00  °¤··ü···»··Þ/···
 r8 0x2c4402a3f6b0d1a5  3189677339326861733
 r9 0x00000000000000c0  192
r10 0x00000000000000c1  193
r11 0x0000000000000000  0
r12 0x00000000000000a4  164
r13 0x00007ffc068fa568  a4 00 00 00 00 00 00 f0 a0 98 88 6b 75 55 00 00  ¤······ð ··kuU··
r14 0xf0000000000000a4  17293822569102704804
r15 0x0000000000000000  0
rip 0x00007f2fde1d26e8  0f 0b 48 8d 35 7f 0c fb ff 48 8d 55 a0 45 31 e4  ··H·5··ûÿH·U E1ä

rflags 0x0000000000010206  PF

cs 0x0033  fs 0x0000  gs 0x0000


Images (17 omitted):

0x00005575320aa000–0x00005575320aafc0 975c0e65c5e0709a2e245a1cca516ca249c0210d a               /tmp/a
0x00007f2fdc914000–0x00007f2fdcaa8e59 2f722da304c0a508c891285e6840199c35019c8d libc.so.6       /usr/lib/libc.so.6
0x00007f2fdde3a000–0x00007f2fde3e6aa0 0f36689015b7c2d06e99375b8216fa8d580d1774 libswiftCore.so /usr/lib/swift/lib/swift/linux/libswiftCore.so

Backtrace took 0.02s

Zig (requires explicit close, but ignores errorrs)

https://ziglang.org/documentation/0.15.2/std/#std.posix.close

Programs

cp (shows error, status code)

Output:

cp: failed to close '/tmp/mnt/badfile': Input/output error

ffmpeg (shows error, status code)

Output:

$ ffmpeg -y -loglevel error -i /tmp/empty.mp4 /tmp/mnt/badfile.mp4
[out#0/mp4 @ 0x555aab40fa80] Error closing file: Input/output error

Firefox (shows error in Downloads)

Gimp (shows error in modal dialog)

ImageMagick (shows error, status code)

Output:

$ magick image.png /tmp/mnt/badfile
magick: unable to write file `/tmp/mnt/badfile' @ error/constitute.c/WriteImage/1443.

IntelliJ Idea (shows error in modal dialog)

Vim (shows error, prevents exit)

Error:

"mnt/badfile" E512: Close failed
WARNING: Original file may be lost or damaged
don't quit the editor until the file is successfully written!

[joshtriplett]

We discussed this at length in today's @rust-lang/libs-api meeting.

We emphatically did not come to any consensus, other than a very clear consensus that this is a painfully broken behavior of NFS and that close should never have been allowed to return errors other than EBADF.

We retreaded a lot of the territory also explored by https://github.com/bytecodealliance/rustix/issues/1084 and https://github.com/rust-lang/rust/issues/98338 .

@sunfishcode's comments on both of the above-linked issues resonated with many on the team:

If try_close exists, it would arguably imply that we believe close can fail, which would arguably imply that all Rust code ever should be using it. I don't think a Rust ecosystem-wide push to eliminate all File::drop calls throughout all Rust code ever would be worthwhile, with what I know of the situation today, and I don't know where else to draw the line.

If we change that, it will create a new expectation for almost all Rust libraries and utilities that write files. It's tempting to say "you don't need to check if your use case doesn't need it", but libraries and utilities usually don't pick the filesystems they run on, or how data is used outside their scope.

We do not want to create the expectation that people are expected to call or use this. And no matter how many warnings we put in comments on a standard library implementation, we would still likely create an ecosystem expectation of migrating to using it.

[the8472]

For those who are specifically targeting NFS on linux, maybe they can rig a flush-to-server-but-not-fsync by using fcntl.

https://manpages.debian.org/unstable/nfs-common/nfs.5.en.html#The_sync_mount_option

[...] the NFS client delays sending application writes to the server until any of these events occur:

• Memory pressure forces reclamation of system memory resources.
• An application flushes file data explicitly with sync(2), msync(2), or fsync(3).
• An application closes a file with close(2).
• The file is locked/unlocked via fcntl(2).

Not tested, purely based on the docs.

[LegionMammal978]

And no matter how many warnings we put in comments on a standard library implementation, we would still likely create an ecosystem expectation of migrating to using it.

Would it, though? Currently, the status quo is that there is no guarantee that the file is written at all unless File::sync_all() returns successfully (and even then, it's more of a suggestion than a guarantee on some OSes), and we even have a warning right at the top of the File documentation, yet somehow, we don't see an ecosystem expectation that every file be synced. So I don't see why it should be so impossible to lump File::close() into the same bucket of "tools to use when you really care about durability and want diagnostics about durability failures".

At worst, one might say that "close() failures are a matter of self-consistency, while sync_all() failures are only a matter of consistency across reboots", but omitting syncs can similarly cause self-consistency issues when the disk cache is flushed.

[tgross35]

I feel like it would be worth it for somebody interested to pitch the mailing list on the possibility of a new fnctl flag for "sync and check errors only for filesystems that may error on close", which then we could consider exposing. Along the lines of what @the8472 mentioned, except without doing an unneeded lock.

[joshtriplett]

Currently, the status quo is that there is no guarantee that the file is written at all unless File::sync_all() returns successfully (and even then, it's more of a suggestion than a guarantee on some OSes), and we even have a warning right at the top of the File documentation, yet somehow, we don't see an ecosystem expectation that every file be synced.

Once people know what fsync does, most use cases don't want it. (If a "barrier" mechanism existed, people might be more inclined to use that much more widely, but OSes don't have a portable mechanism for that.) There are critically important use cases for fsync, but it isn't something that makes sense for most programs in most situations.

So I don't see why it should be so impossible to lump File::close() into the same bucket of "tools to use when you really care about durability and want diagnostics about durability failures".

It's not hard to lump it in that bucket, which is a problem, because it doesn't belong in that bucket. If you care about actual durability (rather than getting diagnostics from particular awful filesystem implementations), it's the wrong tool. And the problem with it being "tools to use when you ... want diagnostics" is that the person writing the code is not the same person who might want the diagnostics, the use case that needs the diagnostics is an intersection of multiple corner cases, and the impact on the ecosystem is massive.

[tbu-]

I feel like it would be worth it for somebody interested to pitch the mailing list on the possibility of a new fnctl flag for "sync and check errors only for filesystems that may error on close", which then we could consider exposing.

That seems like a really weird dance around just exposing close(2). And I guess that's what we'd get as a response from that mailing list: Why create a new API to get the error that close(2) would return when we already have close(2).

[tgross35]

I feel like it would be worth it for somebody interested to pitch the mailing list on the possibility of a new fnctl flag for "sync and check errors only for filesystems that may error on close", which then we could consider exposing.

That seems like a really weird dance around just exposing close(2). And I guess that's what we'd get as a response from that mailing list: Why create a new API to get the error that close(2) would return when we already have close(2).

I think the discussion here is showing that close is not the right tool for the job. What users working on maybe-NFS systems really want is something in between fflush and fsync, which guarantees data has moved to the point that you won't get a recoverable error (i.e. no network failure), but doesn't force a drastic action like flushing to disk when it isn't necessary. close certainly doesn't belong between flush and sync, and using F_SETLK would be a really weird hammer with other drawbacks.

Seems like something that could be generally useful too if you're working with data that might be backed by NFS, to ensure that partial data chunks make it through while keeping the fd open.

[the8472]

And I guess that's what we'd get as a response from that mailing list: Why create a new API to get the error that close(2) would return when we already have close(2).

That seems like unfounded speculation, it wouldn't be consistent with previous statements from kernel devs that close shouldn't return errors. https://lwn.net/Articles/576518/ If it shouldn't, and thus shouldn't be used for error-handling there ought to be an alternative. Now, getting an alternative might still be work and require persuasion, but well, that's the same here.

And we do prefer having APIs with sane semantics.

[tbu-]

That seems like unfounded speculation, it wouldn't be consistent with previous statements from kernel devs that close shouldn't return errors. https://lwn.net/Articles/576518/ If it shouldn't, and thus shouldn't be used for error-handling there ought to be an alternative. Now, getting an alternative might still be work and require persuasion, but well, that's the same here.

And we do prefer having APIs with sane semantics.

Thanks for interesting link. :)