std: Modify TempDir to not fail on drop. Closes #12628

After discussion with Alex, we think the proper policy is for dtors
to not fail. This is consistent with C++. BufferedWriter already
does this, so this patch modifies TempDir to not fail in the dtor,
adding a `close` method for handling errors on destruction.

2 files changed, 47 insertions, 10 deletions

diff --git a/src/libstd/io/buffered.rs b/src/libstd/io/buffered.rs
index a8e7b324bd7..68cbdd2e0aa 100644
--- a/src/libstd/io/buffered.rs
+++ b/src/libstd/io/buffered.rs
@@ -209,7 +209,7 @@ impl<W: Writer> Writer for BufferedWriter<W> {
 impl<W: Writer> Drop for BufferedWriter<W> {
     fn drop(&mut self) {
         if self.inner.is_some() {
-            // FIXME(#12628): should this error be ignored?
+            // dtors should not fail, so we ignore a failed flush
             let _ = self.flush_buf();
         }
     }
@@ -370,6 +370,7 @@ mod test {
     use io;
     use prelude::*;
     use super::*;
+    use super::super::{IoResult, EndOfFile};
     use super::super::mem::{MemReader, MemWriter, BufReader};
     use self::test::Bencher;
     use str::StrSlice;
@@ -584,6 +585,24 @@ mod test {
         assert_eq!(it.next(), None);
     }
 
+    #[test]
+    #[should_fail]
+    fn dont_fail_in_drop_on_failed_flush() {
+        struct FailFlushWriter;
+
+        impl Writer for FailFlushWriter {
+            fn write(&mut self, _buf: &[u8]) -> IoResult<()> { Ok(()) }
+            fn flush(&mut self) -> IoResult<()> { Err(io::standard_error(EndOfFile)) }
+        }
+
+        let writer = FailFlushWriter;
+        let _writer = BufferedWriter::new(writer);
+
+        // Trigger failure. If writer fails *again* due to the flush
+        // error then the process will abort.
+        fail!();
+    }
+
     #[bench]
     fn bench_buffered_reader(b: &mut Bencher) {
         b.iter(|| {
diff --git a/src/libstd/io/tempfile.rs b/src/libstd/io/tempfile.rs
index 8c28caa988a..b4fb95c8af7 100644
--- a/src/libstd/io/tempfile.rs
+++ b/src/libstd/io/tempfile.rs
@@ -10,7 +10,7 @@
 
 //! Temporary files and directories
 
-use io::fs;
+use io::{fs, IoResult};
 use io;
 use iter::{Iterator, range};
 use libc;
@@ -18,13 +18,14 @@ use ops::Drop;
 use option::{Option, None, Some};
 use os;
 use path::{Path, GenericPath};
-use result::{Ok, Err, ResultUnwrap};
+use result::{Ok, Err};
 use sync::atomics;
 
 /// A wrapper for a path to temporary directory implementing automatic
 /// scope-based deletion.
 pub struct TempDir {
-    path: Option<Path>
+    path: Option<Path>,
+    disarmed: bool
 }
 
 impl TempDir {
@@ -48,7 +49,7 @@ impl TempDir {
             let p = tmpdir.join(filename);
             match fs::mkdir(&p, io::UserRWX) {
                 Err(..) => {}
-                Ok(()) => return Some(TempDir { path: Some(p) })
+                Ok(()) => return Some(TempDir { path: Some(p), disarmed: false })
             }
         }
         None
@@ -75,15 +76,32 @@ impl TempDir {
     pub fn path<'a>(&'a self) -> &'a Path {
         self.path.get_ref()
     }
+
+    /// Close and remove the temporary directory
+    ///
+    /// Although `TempDir` removes the directory on drop, in the destructor
+    /// any errors are ignored. To detect errors cleaning up the temporary
+    /// directory, call `close` instead.
+    pub fn close(mut self) -> IoResult<()> {
+        self.cleanup_dir()
+    }
+
+    fn cleanup_dir(&mut self) -> IoResult<()> {
+        assert!(!self.disarmed);
+        self.disarmed = true;
+        match self.path {
+            Some(ref p) => {
+                fs::rmdir_recursive(p)
+            }
+            None => Ok(())
+        }
+    }
 }
 
 impl Drop for TempDir {
     fn drop(&mut self) {
-        for path in self.path.iter() {
-            if path.exists() {
-                // FIXME: is failing the right thing to do?
-                fs::rmdir_recursive(path).unwrap();
-            }
+        if !self.disarmed {
+            let _ = self.cleanup_dir();
         }
     }
 }