Skip to content
This repository has been archived by the owner on Aug 20, 2021. It is now read-only.

option to keep a temporary directory after panic, for debugging purposes #36

Closed
ExpHP opened this issue Oct 5, 2017 · 9 comments
Closed

Comments

@ExpHP
Copy link

ExpHP commented Oct 5, 2017

Suppose that I have some code which works inside of a temp dir, and a bug is detected which causes it to panic. When this occurs, I would like to be able to go look at the temporary directory and inspect the files created leading up to the error. However, during the panic, it will invariably have unwound through the TempDir destructor, and so the directory is gone.

Currently the only real solution I have is to write all of my temp dir construction like this:

let tmp = TempDir::new("my-thing");
let tmp = tmp.path();

and then after a crash occurs, I find the line for the pertinent temp dir, change path to into_path, and then try to reproduce the crash. I don't think this strategy will scale very well.

@KodrAus
Copy link
Contributor

KodrAus commented Oct 5, 2017

Personally I think the cases where you'd expect a temporary directory not to be cleared are application specific enough that they're best handled by your application, building an abstraction around TempDir that persists it in debug builds on drop or something like that. Then you could have something roughly like:

struct Directory {
    inner: Option<TempDir>
}

impl Drop for Directory {
    fn drop(&mut self) {
        if let Some(dir) = mem::replace(&mut self.inner, None) {
            #[cfg(debug_assertions)]
            dir.into_path();
        }
    }
}

@ExpHP
Copy link
Author

ExpHP commented Oct 5, 2017

I took a similar approach to making my life easier shortly after making the issue.

Personally I think the cases where you'd expect a temporary directory not to be cleared are application specific enough that they're best handled by your application

Further making your point, the conditions I chose to leak it on were completely different from yours. I very much want this in release builds!

/// Leaks the inner TempDir if we are unwinding.
impl Drop for TempDir {
    fn drop(&mut self) {
        if ::std::thread::panicking() {
            ::std::mem::forget(self.0.take())
        }
    }
}

But I do think that panicking is special in this regard. There are many ways you could modify the behavior conditionally on debug_assertions without creating a wrapper type (e.g. a fn() -> Box<AsRef<Path>> that conditionally returns a TempDir or a PathBuf), but there's absolutely no other way to modify the behavior conditionally on unwinding.

@KodrAus
Copy link
Contributor

KodrAus commented Oct 6, 2017

Yeh, I see what you mean there. The main thing making me unsure about whether we should support this as a first-class thing in tempdir are other erroneous situations besides panicking where a user might want similar behaviour or not, or do something besides not deleting the directory.

I definitely think we should at least add an example demonstrating how you can avoid dropping the directory when somebody panics like you've shown.

@otavio
Copy link

otavio commented Jan 13, 2018

@ExpHP
Copy link
Author

ExpHP commented Jan 13, 2018

@otavio: But that does not delete the directory on success, almost entirely defeating the point of making a temp dir (the only remaining advantage being that you don't have to name it).

The snippet I linked earlier is an example of the desired behavior, but it is a bit beefy because it makes an effort to do everything that TempDir can. To slim it down a bit:

//! Shim around the `tempdir` crate which doesn't delete the
//! directories on unwind, in order to facilitate debugging.
extern crate tempdir;
pub use tempdir::TempDir as ActualTempDir;

use ::std::io::Result;
use ::std::path::{Path, PathBuf};

/// Wrapper around `tempdir::TempDir` that does not destroy the directory on unwind.
#[derive(Debug)]
pub struct TempDir(Option<ActualTempDir>);

// Forward inherent methods to the tempdir crate.
impl TempDir {
    pub fn new(prefix: &str) -> Result<TempDir>
    { ActualTempDir::new(prefix).map(Some).map(TempDir) }

    pub fn path(&self) -> &Path
    { self.0.as_ref().unwrap().path() }
}

/// Leaks the inner TempDir if we are unwinding.
impl Drop for TempDir {
    fn drop(&mut self) {
        if ::std::thread::panicking() {
            ::std::mem::forget(self.0.take())
        }
    }
}

@otavio
Copy link

otavio commented Jan 14, 2018

I understand but I am unsure it is something we would like to have in production. Possibly, it might be a flag we could configure to enable/disable this behavior.

@ExpHP
Copy link
Author

ExpHP commented Jan 14, 2018

I understand but I am unsure it is something we would like to have in production.

What is the "it" you are referring to? The behavior of leaking temp directories, or the presence of a wrapper type around TempDir?

If it is the former, that is a problem that can be easily solved by anybody who wants to use this solution in their application. They could easily put the body of the drop impl in a conditionally compiled block.

(in fact, this seems a perfect example of the kind of issue @KodrAus is talking about, which shows why this functionality shouldn't be built into the tempdir crate itself. Some applications might want to #[cfg(test)] while others may want to e.g. add a feature for the behavior.)

@otavio
Copy link

otavio commented Jan 15, 2018

@ExpHP yes, leaking temp directories.

That's exactly what I had in mind, someone desiring it would have it in a feature or cfg flag.

@ExpHP
Copy link
Author

ExpHP commented Jan 15, 2018

Okay. I am closing this because there is a clear case for not building this functionality into the crate, and because I do not feel that the documentation needs to show a solution like this. I feel that somebody who is motivated enough can easily figure it out, and that the presence of the example could create unnecessary confusion.

@ExpHP ExpHP closed this as completed Jan 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants