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

Panic on error in TempDir destructor #6

Closed
wants to merge 1 commit into from
Closed

Panic on error in TempDir destructor #6

wants to merge 1 commit into from

Conversation

cmbrandenburg
Copy link

This commit eliminates the silent squashing of most errors in the TempDir
destructor. The NotFound error is still ignored.

This resolves issue #5.

This commit eliminates the silent squashing of most errors in the `TempDir`
destructor. The `NotFound` error is still ignored.
@alexcrichton
Copy link
Contributor

Thanks for the PR! You can find some more background on some Rust issues (rust-lang/rust#14215 and rust-lang/rust#12628), but in general the current convention is for types should strive to never panic in their destructors. As a result this is currently intentional that the destructor does not panic and the error is ignored.

In the case of lingering temporary directories, this sort of situation is always inevitable if, for example, a process is killed or the power goes out, so it's not unique to just ignoring the error on drop here.

As this is intentional, however, I'm going to close this.

@cmbrandenburg
Copy link
Author

Thank you for the quick response and taking the time to explain the Rust convention of not panicking in a destructor. I won't argue against following this or any other convention for your project, so closing this issue is the right thing to do. My complaint is with the convention, not your project.

Nevertheless, I take issue with the examples you give in your second paragraph. Both scenarios—killed, power loss—result in a leaked temporary directory but also my Rust process exiting immediately. Presumably if I'm writing a fault-tolerant program then I've taken care to recover from such graceless exits, so the leaks aren't a big deal in those cases.

Leaks that are a big deal are the ones that occur without my process crashing. In those cases my process enters a degraded state, and that's bad, bad, bad. For example, suppose my temporary directory is a RAM drive. Leaks are as bad as a memory leak, and furthermore, if the drive fills up then the whole system may become unstable. Again, the problem isn't that this can happen—it probably can't be prevented in all cases—but that my process may not receive any indication that it has degraded and therefore may be causing the problem. Such “blind degradation” makes fault-tolerance harder to achieve.

Anyway, my point here is that the two examples you give in your second paragraph are a reason to choose panicking-in-the-destructor over error-squashing. If you need to write crash-resilient programs anyway then panicking is just another way for the process to exit validly. But error-squashing makes it hard for processes to figure out they've degraded, and many real-world programs have a requirement to resist degradation.

opilar pushed a commit to opilar/tempdir that referenced this pull request Sep 22, 2017
Add ability to sort the walked entries
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants