extra::tempfile::TempDir causes IoError during Drop

[klutzy]

use std::os;
use extra::tempfile::TempDir;
fn main() {
    let temp_dir = TempDir::new("asdf").unwrap();
    os::change_dir(temp_dir.path());
}

task '<main>' failed at 'Unhandled condition: io_error:
io::IoError{kind: OtherIoError, desc: "resource busy or locked", detail: None}',

TempDir has Drop implementation of calling rmdir the dir, but it may fail (platform-dependent) if the task change_dired.
part of #10452: run-pass/glob-std.rs.

[klutzy]

I initially thought this would work:

 pub struct TempDir {
     priv path: Option<Path>
+    priv tmpdir: ~Path
 }
...

     pub fn new_in(tmpdir: &Path, suffix: &str) -> Option<TempDir> {
...
-                Ok(()) => return Some(TempDir { path: Some(p) })
+                Ok(()) => return Some(TempDir { path: Some(p), tmpdir: tmpdir.clone() })
...
     }
...
 impl Drop for TempDir {
     fn drop(&mut self) {
        for path in self.path.iter() {
            if path.exists() {
+               os::change_dir(tmpdir);
                fs::rmdir_recursive(path);
            }
        }
     }

but I realized that os::change_dir is not task-safe: #10463

[alexcrichton]

It would seem to me that the most reasonable thing to do would to be just ignore the error in the destructor of TempDir. It's also a real possibility that someone else removed it, but it's just a temporary thing that should do its best to go away. If you changed directories into that directory, then I think that it's then your responsibility for removing it.

[ben0x539]

I don't think unconditionally changing directories in the destructor is appropriate, but I'm not happy about silently leaving temporary directories lying around.

The destructor could check whether the current working directory is a subdirectory of the one it's going to delete and only if so change directory to its parent directory, but then we're still transparently changing directories in a destructor, which I'm pretty sure is going to catch most people by surprise.

I'd suggest we should just fail on all platforms in that case, but then we're failing in a destructor and callers pretty much need to do their own RAII for chdir'ing into and out of the temp dir to avoid unwinding while still in the temp dir and then aborting. :(

[klutzy]

I started to think the real issue is that user does not easily recognize TempDir::drop() will occur. What about adding tempfile::with_temp_dir() function like this? (not tested)

fn with_temp_dir<T>(base: &Path, suffix: &str, f: |temp_dir: &Path| -> T) -> T {
    let prev_path = os::getcwd();
    let temp_dir = TempDir::new_in(base, suffix);
    os::change_dir(temp_dir.path());
    let result = f(temp_dir.path());
    os::change_dir(prev_path);
    fs::rmdir_recursive(temp_dir);
    result
}

[alexcrichton]

cc #12628

[alexcrichton]

Since opening the drop implementation of TempDir has been modified to respect the semantics of not panicking on drop and ignoring I/O errors, so I'm going to close this.