Hacker News Reader

Comment by david2ndaccount | Hacker News Reader

I can’t believe that people still use opening/closing files as examples of RAII when that is one of the cases that RAII can’t handle correctly. A call to close is the last chance the operating system has to signal that an error occurred when writing. You can’t easily signal errors in destructors. In C++, you basically can’t throw an exception and there is no return value from a destructor. Rust suffers the same kind of problem. The only way to properly close a file is to use regular old explicit control flow with if statements. Then you can handle the errors in the normal way.

View on HN

Handling an error from close is a bad way to handle the error, because the file is already closed, so you are pretty much stuffed.

You want to call fsync() instead, and grab the error there, which will prevent you from being able to get an error from close() - see "Dealing with error returns from close()" in https://www.man7.org/linux/man-pages/man2/close.2.html

If you get it on close, you have ~no idea which operation failed.

Obviously doing it on the operation allows you to wrap it up in the handling of the operation and allows you to do RAII (or DIRR as it probably should be known).

(The EBADF option is nonsense in a properly developed application, and should cause a panic. The EINTR position is just a mess.)

Of course, other flow control options are available which also prevent this mess - e.g:

  int failed = with_file("foo", |file|{
       write(file, "bar");
  });

  fn with_file(char* foo, lambda: Fn(File)){
    let f = open(foo);
    if !f { return f; }
    let err = lamda(f);
    let cerr = close(f);
    if !err {
       return err;
    }
    return cerr;
  }

Is this Rust or a different language?

Looks like a mix of Rust and C. Like a French person trying to speak English, but still saying half of the words in French.

I was aiming for pseudo-code. You can write it in C, but it's a little less readable due to the lack of first class lamdas.

Error-on-close is basically another form of error-on-destruct, and like you say it doesn't make much sense to raise an error about something that no longer exists. It's not RAII that's wrong here, but the messy semantics of close() and its use as an ad-hoc asynchronous error reporting mechanism.

There is a right way to do this using RAII, and it works for files and for any other resource that might fail on close: you simply explicitly do the close yourself, but only along the success/happy path. That way you get the best of both worlds: automatic clean-up (ignoring errors) along error paths, and proper close-and-check-for-errors along the happy path.

Of course it'd be even nicer if the implicit RAII close could somehow add its error to the error you are currently propagating, but that's beside the point for your example.

Sure, but defer still fairs worse than RAII does even in this example. In this particular example, yes, you'll need to call flush yourself and handle the error yourself: which you'd still need in a GC'd lang/lang w/ "defer", in addition to remembering to close the file (which the RAII example will have your back on).

> The only way to properly close a file is to use regular old explicit control flow with if statements. Then you can handle the errors in the normal way.

Other than crashing the program, what could you possibly do?

This is what portable software has to work with [1]:

       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. If an I/O error occurred while reading from or writing to the file system during close(), it may return -1 with errno set to [EIO]; if this error is returned, the state of fildes is unspecified.

And it's not just the standard being dumb. Here's what Linux's manual page says about it [2]:

       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().  POSIX.1 is currently silent on this
       point, but there are plans to mandate this behavior in the next
       major release of the standard.

       A careful programmer who wants to know about I/O errors may
       precede close() with a call to fsync(2).

       The EINTR error is a somewhat special case.  Regarding the EINTR
       error, POSIX.1-2008 says:

              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.

       This permits the behavior that occurs on Linux and many other
       implementations, where, as with other errors that may be reported
       by close(), the file descriptor is guaranteed to be closed.
       However, it also permits another possibility: that the
       implementation returns an EINTR error and keeps the file
       descriptor open.  (According to its documentation, HP-UX's
       close() does this.)  The caller must then once more use close()
       to close the file descriptor, to avoid file descriptor leaks.
       This divergence in implementation behaviors provides a difficult
       hurdle for portable applications, since on many implementations,
       close() must not be called again after an EINTR error, and on at
       least one, close() must be called again.

And, just to finish it off, Windows is completely silent on this matter. It mentions EBADF, and that's it [3]. The CloseHandle() function, which POSIX close() (a.k.a. _close()) uses under the hood, also doesn't seem to say anything about whether CloseHandle() failing on a file leaves the handle opened, closed, or in limbo.

And since all of these documentations reserve the right to not actually flush the file when it's closed, any warnings or errors are going to be "best-effort" anyway. If you really want to make sure the file actually gets written, call fsync().

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/c...

[2]: https://www.man7.org/linux/man-pages/man2/close.2.html

[3]: https://docs.microsoft.com/en-us/cpp/c-runtime-library/refer...

> 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

Imagine how tricky it could be to have to debug this or try to reproduce it? Quite the nasty subtle buggy boundary case, especially in Golang where cleanup deferrment is encouraged and touted in lots of example code.

It's easy to imagine a segment of naive language users completely unknowingly risking creating environments where such a bug can thrive.

It goes like this: first deferring the call to close after opening a file and not receiving an error, then calling close again at the end of the happy path "just to be sure" or because "now I'm done with it". And then blundering along their merry way, believing there is nothing wrong with this and "go has my back".

(1) Common File API abstractions that taste more or less like OOPcombined with (2) defer-capable languages (like Go) where this is the "best practice": This class of bug should be considered common and expected.

The tricky part lies in the gap between being newbie-friendly vs. correct and safe. Language makers can't have it both ways.

Maybe static code analysis could detect it and refuse to compile, as it should be considered a foot-gun type of error.

p.s. notriddle: thank you for inlining complete-ish relevant detailed information in your post. It's nice of you.

> ... especially in Golang where cleanup deferrment is encouraged and touted in lots of example code

Go's os.File is not a plain file descriptor. It can be safely closed more than once and will close the underlying FD only the first time the .Close() method is called.

Reply on news.ycombinator.com