io error handling design

[kjpgit]

thanks for reading!

[sinistersnare]

-1 this kinda disagrees with Rust's RAII in my opinion.

[untitaker]

I like the ideas in this, but am not sure at all if this should be applied to all kinds of Writers. I do think it is absolutely necessary for some Writers to force explicit errorhandling on close()

[untitaker]

Previous discussion: https://github.com/rust-lang/rfcs/pull/576/files#r23805711

[untitaker]

@sinistersnare I think the link above explains why RAII doesn't work for some kinds of Writers.

[kjpgit]

@sinistersnare RAII is a great way to release resources and that's what we're doing. An unflushed buffer in BufferWriter is being released. But what we're not doing is attempting to create new resources, e.g. on the filesystem, with no way to relay errors because we don't want to panic in a destructor.

[mahkoh]

-1

The current way works well in practice which means that this RFC is a serious step backwards for most users.

[sinistersnare]

I feel like the sort of random panics that are trying to be prevented in this RFC are still in Rust in a few ways. For example, println! can panic, indexing, and a few other things. I think Mitsuhiko had an article about this at one point.

Basically, I do not think that the correctness benefit is greater than the ergonomic loss. However, if there is a resounding yes to this proposal, I am no language designer (I wish I was, but alas), so I wont be too upset it if was accepted.

Thanks for the reading, I am currently procrastinating homework, but I will try to read that later :)

[untitaker]

I agree with you, but IMO it's less problematic with println, because if you want to explicitly handle errors, stdout.write is not too low-level.

I suppose ideally Rust would have an API with explicit error checks, and one for the common usecase which panics on edgecases.

[Diggsey]

There was a discussion about the same issue but for C++: the ideal semantics IMO, would be for the guard to flush and close the underlying stream iff it exits the scope normally, rather than as the result of a panic and stack unwinding. Errors from close result in a panic (to avoid this, close must be called explicitly and the errors handled)

The problem is that it's impossible to implement this correctly using only a library method such as "is thread panicking" to be called from the destructor to determine whether to close() or not, because even during a panic, a guard within a destructor can exit scope normally.

Possible implementation: add a "Closable" trait. The compiler will automatically insert calls to "close()" when a stack object implementing that trait leaves scope normally (ie. not as part of unwinding).

For Closable objects constructed on the heap, it is up to the owner to call "close" explicitly. For example, Box<T: Closable> should itself implement Closable and forward on the calls as expected.

[kjpgit]

Thanks @Diggsey for that comment about a guard in a destructor. I added a note "Note that this can give a false negative" in the RFC, but I am doing the reverse logic as what you are suggesting.

And +1 to your comment. I'm trying to decide if it's backwards compatible with my proposal. We could say implicit drop has undefined behavior if you didn't call close(), and leave it at that until Closeable happens, and then it changes to defined behavior - nothing happens unless it left scope normally, in which case it is closed, but then that could in theory cause panics in code that didn't before.

[kjpgit]

@Diggsey the way to stop Closeable would be to explicit drop(myfile), right? In case you wanted to be future proof. e.g. in pseudocode:

do_something_with(&my_file)
if all_ok: my_file.close()
else: drop(my_file)

I'm assuming there is potentially a use case somewhere where you don't want to flush and close the buffered file in some circumstance.

[Diggsey]

@kjpgit Yeah, anything which consumes the Closable would prevent the compiler from emitting calls to close or drop as usual. The drop method itself shouldn't call close either, so that gives you a way to avoid it.

I think adding Closeable later can be made backwards compatible, as long as a few conditions hold:

1. Current code always calls either "close()" or "drop()" explicitly before the guard goes out of scope (assuming no panics). This can be enforced with a lint. (and this lint should probably exist anyway even if Closeable is never added)
2. Calling "close()" multiple times is either a no-op, or impossible because it consumes self.

[kjpgit]

@Diggsey I'm still confused how the automagic scoped_close() method is prevented. drop is literally an empty function, and we want to move file objects around to new scopes. So both the close() method itself needs a way to take ownership, close resources, and drop without having the scope_close() called again. Same with the example code I did earlier. There seems to be a more magical drop() needed.

[Diggsey]

Sorry I phrased that badly, when I said "drop" shouldn't call close, I meant it should be defined to not call close, not that it will already behave that way under the current implementation.

[kjpgit]

@Diggsey I think the Closeable will unfortunately cause panics with people who are trying not to write panicing code. E.g.

try!(my_file.close());
try!(my_file2.close());

Is obviously designed not to panic, but if Closeable is the default, if my_file fails and try! tries to return an error, and then my_file2 also fails autoclose, it will cause a panic where there shouldn't have been one.

[kjpgit]

@mahkoh "The reason why rust "somewhat" works today is because Rust's stdout
is line buffered all the time. When that changes to be fully buffered, except
to tty, that means even println!() panic semantics are totally useless, because
nothing will be written until flush/close/drop."

[Diggsey]

@kjpgit That's a difficult case - I'm starting to think that explicit close might be the best we can do. In combination with a lint it shouldn't be too bad.

[mahkoh]

@kjpgit: I consider panic quite useless and would not mind having it replaced by abort. Either way, you wouldn't use println! and friends outside of toy programs.

[mzabaluev]

I think the use case to have close as a generic trait method should be illustrated by some real life examples. For some applications, into_inner semantics might be more useful: e.g. you want to finalize a data format writer, but retain the underlying sink for further writing. Closing a stdio stream is something that should not be done casually, and at the same time it's a valid operation to perform on the underlying POSIX file descriptor, useful in certain cases. In general, the code where an I/O 'pipeline' is built is usually responsible for finalization, in which case there is no need for generic or dynamically dispatched methods.

[mzabaluev]

To clarify the comment above: I support the need for close on File, TcpStream, etc., and I agree with the self-consuming signature because it follows the POSIX semantics. But I don't think it should be part of Write. It may need its own trait to implement a generic RAII guard, or the destructors of types that have close may simply perform the necessary cleanup without checking errors (which is the moral equivalent of calling libc::close() on Linux without error-checking: you free the descriptor and its kernel resources, but ignore the possible data loss).

[mzabaluev]

Making the types that need explicit close to be linear types (#814) may help sidestep the issue of silent failures in destructors.

[aturon]

When we discussed this (fairly extensively) during IO reform, one of the basic questions was: what errors can you receive when trying to close a file that would not also arise by flushing it?

That is, in what circumstances is calling flush where you would call close not enough to discover/handle the errors you want to handle?

cc @sfackler

[gkoz]

It's not a file... but a block mode cipher that's had an incomplete block of data written to it can't flush anything unless it's being closed in which case it might, depending on the mode, error out if the end was not block aligned or if the input didn't have correct padding at the end; an authenticated cipher can return an authentication error.

[aturon]

@gkoz That's an interesting example, but it's not clear why flush couldn't generate an error in that case? (Generally, flush returns a Result precisely so that it can report obstructions like that.)

[gkoz]

@aturon Generally, it could work but when doing foreign library bindings (e.g. rust-openssl) it's not always possible. Or rather I'd say openssl actively refuses to play by Rust's rules ;) (We could catch the case of unaligned end of data in block modes relatively easily, tracking padding would require some effort and overhead, the authentication case? well...) Isn't it also a bit odd for flush to return errors just because you're not done writing all of your data yet?

[gkoz]

To rephrase the last point, is it appropriate for flush to double as some kind of can_close that asks if the writer's input-determined state allows it to be closed immediately regardless of the possibility of writing more bytes into it?

[kjpgit]

@aturon I'm confused. Are you inquiring about something besides what the close(2) NOTES section on linux talks about?

[mzabaluev]

The implementations of flush for POSIX descriptors is a no-op. In general, I don't see how it is possible to preempt possible failures of close with anything else.

[alexcrichton]

This RFC is now entering its week-long final comment period.

My personal feelings on this RFC is that while it may certainly be desirable to have a close function on I/O objects the case isn't super compelling to do so right now. The discussion on this RFC has stalled for quite some time now so I think we can revisit it at a later date.

[aturon]

I agree with @alexcrichton; while there is a hypothetical need for this functionality, I think we have much bigger fish to fry with respect to IO, and I would prefer to wait on this until there is a more clear need for the functionality.

It's worth noting that, especially with the forthcoming IntoRaw traits, it should be easy to build this functionality (taking the IO object by value) out of tree.

[kjpgit]

I'm not sure what bigger fish to fry there are than making sure data isn't
truncated/corrupted on posix systems that have EIO during close. But if
you want all code that uses the stdlib to be buggy on posix, including code
that merely uses libraries that internally use the stdlib, I guess it's
your choice.

On Thu, Jul 16, 2015 at 11:03 AM, Aaron Turon notifications@github.com
wrote:

I agree with @alexcrichton https://github.com/alexcrichton; while there
is a hypothetical need for this functionality, I think we have much bigger
fish to fry with respect to IO, and I would prefer to wait on this until
there is a more clear need for the functionality.

It's worth noting that, especially with the forthcoming IntoRaw
#1174 traits, it should be easy
to build this functionality (taking the IO object by value) out of tree.

—
Reply to this email directly or view it on GitHub
#770 (comment).

[mzabaluev]

I, too, think it a pity that one needs more than the standard library to use the basic file functionality on POSIX systems with a degree of reliability that the developers are accustomed to (no hard write-through guarantee, but a number of easy-to-detect errors not going unreported either).

Hiding a potentially blocking operation in the destructor without an explicit way to invoke it or drain the output buffer (files only have fsync which can be overkill, and there is no way to "flush" a socket that works in all cases) does not strike me as a particularly clear design, either.

[alexcrichton]

@kjpgit

But if you want all code that uses the stdlib to be buggy on posix, including code that merely uses libraries that internally use the stdlib, I guess it's your choice.

I think this is a bit hyperbolic to say, so it may not be 100% faithful to this RFC. We will always have primitives close themselves on drop, so receiving an error from the close operation is inevitably going to be an opt-in no matter how we spin it. This sort of functionality is enabled via the mechanisms @aturon said and this will be available on crates.io. If it is ubiquitously used then it's definitely a candidate for being in libstd!

---

@mzabaluev

Hiding a potentially blocking operation in the destructor without an explicit way to invoke it or drain the output buffer (files only have fsync which can be overkill, and there is no way to "flush" a socket that works in all cases) does not strike me as a particularly clear design, either.

I'm not sure this is a line of reasoning we can follow too far though as it's unclear how many possible designs there are in this space. As I mentioned above, we're always going to call close in a destructor, so silently blocking in a destructor is pretty orthogonal to this RFC. Perhaps warning when this happens and providing the ability to see when this happens are related to this, but talking about cases like this which are likely inevitable doesn't seem to change the course of this RFC much.

It seems like we will fundamentally always close I/O objects on drop, and then we will also very likely provide the ability to manually close an object to see if any errors arise. It will always be an opt-in to seeing the error on close, so it's a given that we have to work with.

[tbu-]

@alexcrichton

My personal feelings on this RFC is that while it may certainly be desirable to have a close function on I/O objects the case isn't super compelling to do so right now. The discussion on this RFC has stalled for quite some time now so I think we can revisit it at a later date.

The problem is that right now there isn't an obvious way to write to a file and check whether an error has occured. That's pretty fundamental if you ask me.

To hyperbolize this point a bit, not checking the return value of close is akin to not checking it on write. You might have written everything to the file, but you never know. Not providing the close function if even its man page states that not checking the return value of close is a "serious programming error" is a non-starter for a programming language that usually tries to avoid silent data loss. In other languages, this is probably handled by throwing an exception (C++, Python, ...).

I fail to understand how you can brush this off as a minor issue. :(

[untitaker]

As far as I understand close is panicking in Rust if errors occured. While
this makes sense on normal POSIX filesystems, this is not appropriate behavior
when pointing a Rust program against a FUSE filesystem that might have
completely different semantics.

On Fri, Jul 24, 2015 at 02:59:20PM -0700, tbu- wrote:

@alexcrichton

My personal feelings on this RFC is that while it may certainly be desirable to have a close function on I/O objects the case isn't super compelling to do so right now. The discussion on this RFC has stalled for quite some time now so I think we can revisit it at a later date.

The problem is that right now there isn't an obvious way to write to a file and check whether an error has occured. That's pretty fundamental if you ask me.

To hyperbolize this point a bit, not checking the return value of close is akin to not checking it on write. You might have written everything to the file, but you never know. Not providing the close function if even its man page states that not checking the return value of close is a "serious programming error" is a non-starter for a programming language that usually tries to avoid silent data loss. In other languages, this is probably handled by throwing an exception (C++, Python, ...).

I fail to understand how you can brush this off as a minor issue. :(

---

Reply to this email directly or view it on GitHub:
#770 (comment)

[tbu-]

@untitaker
I think Rust just prints a warning to stderr if the file closing does not succeed.

[main--]

@untitaker @tbu- As there seems to be confusion regarding Rust's current behavior:

• On Unix, most IO resources, including Files (and sockets) are represented as FileDesc. When dropped, it calls close but discards errors due to the reasons outlined in the comment.
• On Windows, it's basically the same story with Handles. The result of CloseHandle isn't checked either. closesocket is no exception.
• Surprisingly, when working with directories on unix, the subsequent closedir call must succeed or else Rust will panic. However this happens only in debug builds.
• Once again, the windows behavior matches (just like it shoud)

[alexcrichton]

The consensus of the libs team this week was to close this RFC. While it's something we may want to add to the standard library in the future, with the addition of IntoRawFd it is possible to develop these primitives externally on crates.io first.

Thanks again for the RFC @kjpgit!