20

I have very weird behavior. I have,

Directory.Delete(tempFolder, true);
if (Directory.Exists(tempFolder))
{
}

Sometimes Directory.Exists return true. Why? May be the explorer is open?

one noa
  • 345
  • 1
  • 3
  • 10
Imran Qadir Baksh - Baloch
  • 32,612
  • 68
  • 179
  • 322

2 Answers2

29

Directory.Delete calls the Windows API function RemoveDirectory. The observed behavior is documented:

The RemoveDirectory function marks a directory for deletion on close. Therefore, the directory is not removed until the last handle to the directory is closed.

The .NET documentation is unfortunately missing this information. Whether the static Directory.Delete method opens a handle to the directory is not documented. Likewise, if it does, it is not documented when the handle is closed.

Without any of this information, the best you can do is to poll for completion:

Directory.Delete(tempFolder, true);
while (Directory.Exists(tempFolder)) Thread.Sleep(0);
// At this point the directory has been removed from the filesystem

Even though polling should generally be avoided in preference of events, installing a filesystem watcher would be a bit over the top for this. Still, keep in mind, that this operation does not come for free, particularly when dealing with a network drive.

Update: With .NET's Reference Source available, the implementation of Directory.Delete can be inspected. The first action of this method is to iterate over all files and delete them. The iteration is implemented using FindFirstFile/FindNextFile. The returned handle is stored as a SafeFindHandle, a concrete subclass of SafeHandle. As the documentation points out, the native handle is freed through a concrete ReleaseHandle override. ReleaseHandle is called from a (postponed) critical finalizer. Since finalization is non-deterministic, this explains the open handle, responsible for the delayed directory delete.

This information, however, does not help in finding a better solution than the one described above (polling for completion).

Other answers to this question did not identify the core issue, and work by coincidence at best. BanksySan's answer adds unrelated code that introduces a delay to allow time for open handles to be closed. Byeni's answer is closer, yet still off: When he talks about the object referencing the directory he almost nails it. However, the object referencing the directory is called a handle, a native resource. Native resources are disposed of in finalizers, and GC.Collect() does not run finalizers. This, too, appears to work by buying extra time.
Community
  • 1
  • 1
IInspectable
  • 46,945
  • 8
  • 85
  • 181
  • 1
    The `SafeFindHandle` is wrapped in a `using`. Are you saying that although `SafeFindHandle` implements `IDisposable`, and the `Dispose()` method gets called, `SafeFindHandle` still saves some of the work for the finalizer? –  Jun 16 '15 at 21:45
  • 2
    @hvd: According to the [documentation](http://referencesource.microsoft.com/#mscorlib/system/runtime/interopservices/safehandle.cs,56) the native handle is freed only after normal finalizers have been run. The `IDisposable` pattern does not run finalizers. – IInspectable Jun 16 '15 at 21:54
  • 1
    That's not the documentation, that's a comment. [The documentation is here.](https://msdn.microsoft.com/en-us/library/system.runtime.interopservices.safehandle%28v=vs.100%29.aspx) The documentation for the `Dispose()` method states "Releases all resources used by the SafeHandle class." The reference source does not contradict this: both the finalizer and `Dispose()` call external code we can't check. MS would be extremely foolish if they did implement `IDisposable` in such an utterly useless way as you describe, where `Dispose()` does not actually release resources. :) –  Jun 16 '15 at 21:59
  • 1
    @hvd: The documentation for [ReleaseHandle](https://msdn.microsoft.com/en-us/library/system.runtime.interopservices.safehandle.releasehandle.aspx) mandates: *"When overridden in a derived class, executes the code required to free the handle."* The reason, why *`IDisposable` is implemented in an utterly useless way* is also [documented](https://msdn.microsoft.com/en-us/library/system.runtime.interopservices.safehandle.aspx): *"a FileStream object can run a normal finalizer to flush out existing buffered data without the risk of the handle being leaked or recycled."* – IInspectable Jun 16 '15 at 22:17
  • You keep pointing out what the finalizers do, but you don't answer the question of whether `Dispose()` does the same thing. You assert that it doesn't, and while I can see how you feel that is supported by the comment block you link to, there still isn't any support for it from either the code or the documentation. Your quote *doesn't* say whether FileStream flushes data during `Dispose()`. It merely says that FileStream flushes data during its finalizer, while leaving open the possibility that other method calls do the same thing. –  Jun 17 '15 at 05:45
  • @hvd: `Dispose()` cleans up **managed resources**, when called with an argument of `true` (ultimately what the `using` statement does). `Dispose()` frees **unmanaged resources**, when called with an argument of `false`. The latter invokes the finalizer. Finalization is performed by the garbage collector, running out-of-band. This implies, that finalizers won't run when `IDisposable::Close()` is executed. This is essentially the reason why we have finalizers to begin with. – IInspectable Jun 17 '15 at 08:04
  • No. Just plain no. `Dispose()` cleans up managed *and* unmanaged resources. The finalizer cleans up any unmanaged resources that hadn't been cleaned up earlier (possibly because the programmer forgot to call `Dispose()`). That's the standard [Dispose Pattern](https://msdn.microsoft.com/en-us/library/b1yfkh5e(v=vs.110).aspx) as documented by Microsoft, anyway. –  Jun 17 '15 at 08:34
  • @hvd: `Dispose()` calls `Dispose(bool)` (by default). The boolean parameter is used to distinguish, whether the method is called from a finalizer, or as part of regular (deterministic) cleanup. In case of the `SafeHandle` implementation, `Dispose()` calls `Dispose(true)`, skipping cleanup of unmanaged resources. If you insist that this is wrong, please consider whipping out WinDbg, and post your own answer to this question, and share your findings. – IInspectable Jun 22 '15 at 09:48
  • The link I gave you showed you the standard Dispose pattern. You're right that it calls `Dispose(true)`, but the standard Dispose pattern as documented on that page releases unmanaged resources independent of the `disposing` parameter, and managed resources only if `disposing` is `true`. Look for `ReleaseBuffer(buffer); // release unmanaged memory`: it isn't in any `if` block. The reference source is not complete enough to answer whether `SafeHandle` implements that, and WinDbg is complicated. But sure, I will (later) try to create a simple test application to see how it behaves at runtime. –  Jun 22 '15 at 10:15
  • If it really is the handle, 'GC.WaitForPendingFinalizers()' should be another solution. But some quick tests have shown that it is reproducable even with Collect and Wait (Even with some thread switch before). So there must be something else that prevents the deletion and polling still remains the only way :( – sanosdole May 31 '16 at 14:08
  • 1
    @sanosdole: See the first quote of my answer: *"The RemoveDirectory function marks a directory for deletion on close. Therefore, the directory is not removed until the last handle to the directory is closed."* `GC.WaitForPendingFinalizers()` still does not ensure, that the directory is deleted immediately. The only guarantee is, that it will be deleted at some point after the final handle to the directory has been closed. – IInspectable May 31 '16 at 14:15
8

Use DirectoryInfo instead, and call Refresh() on that.

        var dir = new DirectoryInfo(tempFolder);
        dir.Delete();
        dir.Refresh();

Because we are performing many operations on the directory, it is more performant to use DirectoryInfo rather that Directory. This probably explains why there is no Refresh() on the static class, it is meant for one off operations and so would never need to be refreshed.

If might be worth adding a Thread.Sleep(0) after the refresh to relinquish the thread and put it to the back of the pool. Haven't tested that though, it's just a musing.

BanksySan
  • 27,362
  • 33
  • 117
  • 216
  • May be a good point, but I tried this to solve the original question, and it does not help. Accepted answer seems to work without fail. – Nik Mar 05 '20 at 15:55