Add the Close trait

text/0000-close-trait.md

@@ -0,0 +1,303 @@
+- Feature Name: close-trait
+- Start Date: 2019-04-03
+- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000)
+- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000)
+
+# Summary
+[summary]: #summary
+
+Add a `Close` trait to `std::io` that allows for dropping a value and producing
+a `Result`.
+
+# Motivation
+[motivation]: #motivation
+
+In many programming languages, it is possible to explicitly `close` many types
+of resources.  In Rust, we have the `Drop` trait to automatically cleanup these
+resources for us.  For various reasons, we have decided that `Drop` shall never
+fail.  Thus errors encountered in a `Drop` implementation should be ignored.
+Implementations of `Close` will allow us to explicitly handle errors that could
+occur in a `Drop` implementation.
+
+Adding the ability to manually `close` resources will allow users of the
+language to handle errors only revealed when the resource is dropped.  One
+example of this would be a race condition for shared resource access.  Another
+would be in the case of a resource that cannot be fully flushed before it is
+dropped.  It may not be possible to completely flush a resource without closing
+it.  It may also be possible that the process of closing a resource itself could
+cause an error.  In this case, there is currently no standard way to deal with
+all of these edge cases.
+
+Making `Close` into a trait will allow us to generically use this functionality
+for many types of resources.  This will also allow us to `close` wrapper types,
+such as `BufReader` or `BufWriter` when they wrap any type implementing `Close`.
+
+For specifically `File`s, we can call `sync_all` to force synchronization of the
+data to the filesystem.  But this function is no longer directly available when
+we use a `BufReader` or `BufWriter`.
+
+# Guide-level explanation
+[guide-level-explanation]: #guide-level-explanation
+
+Currently in rust, we can manipulate files simply and expect them to be
+automatically closed:
+
+```rust
+use std::fs::File;
+use std::io::prelude::*;
+
+fn main() -> std::io::Result<()> {
+    let mut file = File::create("foo.txt")?;
+    file.write_all(b"Hello world!")?;
+    Ok(())
+}
+```
+
+Although this checks if there were errors in writing the data to the file, there
+sometimes are spurious failures that aren't caught here.  Our program is
+reporting no errors and we can't figure out what is going wrong.  To solve this,
+we can call `close` and handling the errors it produces.  This method simply
+communicates its meaning -- close the file.
+
+```rust
+use std::fs::File;
+use std::io::prelude::*;
+use std::io::Close;
+
+fn main() -> std::io::Result<()> {
+    let mut file = File::create("foo.txt")?;
+    file.write_all(b"Hello world!")?;
+    file.close()?;
+    Ok(())
+}
+```
+
+# Reference-level explanation
+[reference-level-explanation]: #reference-level-explanation
+
+This trait is added to `src/io/close.rs`:
+
+```rust
+pub trait Close {
+    type Error;
+
+    fn close(self) -> Result<(), Error>;
+}
+```
+
+As it currently stands, there shall be no default implementation.
+
+## File
+
+This trait will be implemented for `fs::File` with `Error` specified as
+`io::Error`.  Similarly to how other methods are implemented on the inner `File`
+implementations, `close` takes `self.inner` by reference (see below).  Thus we
+must `forget` the `File` to prevent a double free.  To do this we place this
+impl block in `src/libstd/fs.rs`:
+
+```rust
+impl Close for File {
+    type Error = io::Error;
+
+    fn close(self) -> io::Result<()> {
+        let result = self.inner.close();
+        crate::mem::forget(self);
+        result
+    }
+}
+```
+
+## Underlying os-specific behavior
+
+The underlying implementation of `File`s will support a method of the following
+signature: `pub fn close(&self) -> io::Result<()>`.  For most of these
+implementations, this boils down to wrapping `cvt` around the underlying close
+function call and appending `.map(|_| ())` to get rid of the error code.  This
+will call the system-level close function and return errors when applicable.
+For example, on unix, this is `close`.  On Windows, this is `CloseHandle`.
+
+Here is the example unix implementation.  Add this to
+`src/libstd/sys/unix/fs.rs` in the `impl File`:
+
+```rust
+pub fn close(&self) -> io::Result<()> {
+    self.0.close()
+}
+```
+
+In `src/libstd/sys/unix/fd.rs`, add this to the `impl FileDesc`:
+
+```rust
+pub fn close(&self) -> io::Result<()> {
+    cvt(unsafe { libc::close(self.fd) }).map(|_| ())
+}
+```
+
+## BufReader and BufWriter
+
+For `BufReader` and `BufWriter`, we implement `Close` if and only if the
+underlying type implements `Close`.  For `BufReader`, we delegate to the
+underlying `Close` implementation.
+
+```rust
+impl<R: Close> Close for BufReader<R> {
+    type Error = R::Error;
+    fn close(self) -> Result<(), Self::Error> {
+        self.into_inner().close()
+    }
+}
+```
+
+For `BufWriter`, it is possible that flushing our buffer and closing the wrapped
+resource will cause an error.  In this case, we must ignore the error from one
+or the other.  If both cause an error, it is likely that it is from the same
+cause.  I believe the correct choice is to ignore the error in closing the file
+and drop the underlying resource normally.
+
+```rust
+impl<R: Close<Error = io::Error> + Write> Close for BufWriter<R> {
+    type Error = R::Error;
+    fn close(mut self) -> Result<(), Self::Error> {
+        let result = self.flush_buf();
+        let inner = self.inner.take();
+        result?;
+        inner.unwrap().close()
+    }
+}
+```
+
+## TCPStream and UDPSocket
+
+In a very similar way to a `File`, these two structs can implement `Close`.  The
+outer, facade layer implements it by delegating to the inner.  On each operating
+system we implement the `Close` trait for the wrapped version.  The os-specific
+implementations are constructed in nearly the same way as for `File`s.
+
+# Drawbacks
+[drawbacks]: #drawbacks
+
+* This adds more clutter to the standard library.
+* The `Close` trait wouldn't be a part of the prelude and thus could be
+  overlooked.
+* `Close` implementations for wrapper types are often times hard to correctly
+  implement (see `BufWriter`).
+
+# Rationale and alternatives
+[rationale-and-alternatives]: #rationale-and-alternatives
+
+## Alternatives
+
+### As a method of `File`
+
+Although this would lead to less code, it would reduce the applicability of this
+functionality to more modules in the future.  It is common to wrap resources in
+a buffered interface such as `BufReader` or `BufWriter`.  We could add a `close`
+method to these wrappers that delegates to `File::close`.  But this would
+prevent us from easily closing many other types, such as sockets and streams.
+
+We could even have a method in each of these structs that delegates to the
+trait's implementation, allowing for people to `close` resources without
+explicitly importing `Close`.
+
+### Default implementation for `Close`
+
+We could implement `Close` for all `Drop` types by always succeeding.  Since
+this change can be done retroactively later without breaking backwards
+compatibility, I choose to leave it out of this RFC.  Example implementation is
+included in case this is desirable in the future:
+
+    default impl<T> Close for T {
+        type Error = !;
+        fn close(self) -> Result<(), Self::Error> {
+            Ok(())
+        }
+    }
+
+### Named `TryDrop` instead
+
+We could name this trait `TryDrop` instead, and the method `try_drop`.  This
+seems well and good as `Close` allows us to generalize the idea of `Drop`
+similarly to `TryFrom` and `TryInto`.  I do not prefer this so long as `close`
+takes `self` by value because I do not think it is intuitive to have different
+input parameters for semantically similar functions.
+
+    fn try_drop(self) -> Result<(), Error>;
+    fn drop(&mut self) -> ();
+
+### Taking `self` by `&mut`
+
+`Close` has the signature `fn close(self) -> Result<(), Error>` whereas `Drop`
+has `fn drop(&mut self) -> ()`.  These signatures aren't identical, but they
+could be made to be.  We could rework the method to be `fn try_drop(&mut self)
+-> Result<(), Error>`.  This yields a few implementation ideas:
+
+1. Treat `try_drop` like `drop`.  Thus we have a similar wrapper function
+   `std::mem::try_drop` that took it by value.  Make the `try_drop` method
+   uncallable.  This would require a lot of new language features just to
+   achieve this simple library feature.  This function would have to have some
+   compiler magic to recursively `drop` each member after `try_drop` is called
+   without calling the `drop` instance on the overall `struct`.
+
+2. Have this method prepare the object for destruction, effectively making it
+   unusable.  This would allow for `dyn Close` objects to be usable.  It also
+   offers more compatibility with external libraries that already use `&mut
+   self` for similar methods.  However, it also brings some down sides.  It
+   renders the object into a unusable state, where every method called either
+   performs undefined behavior or returns an error.  The first case is bad and
+   goes against Rust's core principles.  The second is bad because it forces the
+   object to provide a special case specifically to handle implementing `Close`.
+   For this reason, I believe implementers will choose not to implement this
+   trait when it is so much easier to not worry about it.
+
+### As a method of Write
+
+I believe this is a bad idea because it would remove backwards compatibility.
+It also prevents us from closing reading resources.
+
+# Prior art
+[prior-art]: #prior-art
+
+C++ is an apt comparison as it also automatically closes files.
+[`std::basic_filebuf`](https://en.cppreference.com/w/cpp/io/basic_filebuf) is
+used as the underlying implementation for files.  The
+[destructor](https://en.cppreference.com/w/cpp/io/basic_filebuf/%7Ebasic_filebuf)
+also ignores errors when closing the file.  It provides a
+[`close`](https://en.cppreference.com/w/cpp/io/basic_filebuf/close) method that
+rethrows exceptions while guaranteeing the file is closed.
+
+In Java, there exists a generic `close` method that yields errors.  This is the
+interface
+[`java.io.Closeable`](https://docs.oracle.com/javase/7/docs/api/java/io/Closeable.html).
+It is implemented by all classes extending either
+[`java.io.Reader`](https://docs.oracle.com/javase/7/docs/api/java/io/Reader.html)
+or
+[`java.io.Writer`](https://docs.oracle.com/javase/7/docs/api/java/io/Writer.html).
+This RFC is very similar in implementation to this interface except that
+instances of `Read` and `Write` are not required to implement `Close`.
+
+The Rust library Tokio has a similar concept.  It defines the method
+[`shutdown`](https://docs.rs/tokio/0.1.18/tokio/io/trait.AsyncWrite.html#tymethod.shutdown)
+as part of their trait `AsyncWrite`.  As described in the documentation there, this allows
+for handling errors in shutting down the stream before the object is dropped.
+It does this by taking `&mut self` and rendering `self` unusable after it is
+called.  A discussion of the tradeoffs of this are made above.
+
+The Rust library futures-preview has a similar concept as Tokio, defining the
+method
+[`poll_close`](https://docs.rs/futures-preview/0.3.0-alpha.13/futures/io/trait.AsyncWrite.html#tymethod.poll_close)
+as part of their trait `AsyncWrite`.  The semantics are the same as Tokio, but
+the name more resembles that of `Close`.
+
+# Unresolved questions
+[unresolved-questions]: #unresolved-questions
+
+What module should this go in?  It seems logical to go in `std::io`, but it
+might be more appropriate for `std::mem` beside `std::mem::drop`.
+
+Should this be named `Close` or `TryDrop`?
+
+# Future possibilities
+[future-possibilities]: #future-possibilities
+
+Provide a default implementation of `Close` for any type deriving `Drop`.  This
+is a backwards compatible change so it can be addressed in a later RFC.