Should Dispose() or Finalize() be used to delete temporary files?

I have a class that makes use of temporary files (Path.GetTempFileName()) while it is active. I want to make sure these files do not remain on the user's hard drive taking up space after my program is c开发者_开发技巧losed. Right now my class has a Close() method which checks if any temporary files used by the class still exist and deletes them.

Would it make more sense to put this code in the Dispose() or Finalize() methods instead?

Better yet would be to create the file with FileOptions.DeleteOnClose. This will ensure that the operating system forcibly deletes the file when your process exits (even in the case of a rude abort). Of course, you will still want to close/delete the file yourself when you are done with it, but this provides a nice backstop to ensure that you don't allow the files to be sit around forever

Example:

using (FileStream fs = File.Create(Path.GetTempFileName(), Int16.MaxValue, 
       FileOptions.DeleteOnClose)) 
{ 

    // Use temp file 

} // The file will be deleted here

I would do both; make the class disposable, and have the finalizer clean it up. There is a standard pattern for doing so safely and effectively: use it rather than attempting to deduce for yourself what the right pattern is. It is very easy to get wrong. Read this carefully:

http://msdn.microsoft.com/en-us/library/system.idisposable.aspx

Note that you've got to be really really careful when writing a finalizer. When the finalizer runs, many of your normal assumptions are wrong:

• There are all kinds of potentials for race conditions or deadlocks because you are no longer on the main thread, you're on the finalizer thread.

• In regular code, if you're running code inside an object then you know that all the things the object refers to are alive. In a finalizer, all the things the object refers to might have just been finalized! Finalizers of dead objects can run in any order, including "child" objects being finalized before "parent" objects.

• In regular code, assigning a reference to an object to a static field could be perfectly sensible. In a finalizer, the reference you are assigning could be to an already dead object, and therefore the assignment brings a dead object back to life. (Because objects referred to by static fields are always alive.) That is an exceedingly weird state to be in and nothing pleasant happens if you do.

• And so on. Be careful. You are expected to fully understand the operation of the garbage collector if you write a non-trivial finalizer.

A file is an unmanaged resource, and you implement IDisposable to clean up unmanaged resources that your classes are dependent upon.

I have implemented similar classes, although never in production code.

However, I understand your tentativeness about this - user interaction with the files outside of your application could screw things up and cause problems during disposal. However, that is the same for any file created/deleted by an application, regardless of whether or not it's tidied up by a Dispose() method or not.

I'd have to say that implementing IDisposable would be a reasonable choice.

A nice way is suggested by David M. Kean on the MSDN entry on Path.GetTempFileName. He creates a wrapper class implementing IDisposable that will automatically remove the file:

public class TemporaryFile : IDisposable
{
    private bool _isDisposed;

    public bool Keep { get; set; }
    public string Path { get; private set; }

    public TemporaryFile() : this(false)
    {
    }

    public TemporaryFile(bool shortLived)
    {
        this.Path = CreateTemporaryFile(shortLived);
    }

    ~TemporaryFile()
    {
        Dispose(false);
    }

    public void Dispose()
    {
        Dispose(false);
        GC.SuppressFinalize(this);
    } 

    protected virtual void Dispose(bool disposing)
    {
        if (!_isDisposed)
        {
            _isDisposed = true;

            if (!this.Keep)
            {
                TryDelete();   
            }
        }
    }

    private void TryDelete()
    {
        try
        {
            File.Delete(this.Path);
        }
        catch (IOException)
        {
        }
        catch (UnauthorizedAccessException)
        {
        }
    }

    public static string CreateTemporaryFile(bool shortLived)
    {
        string temporaryFile = System.IO.Path.GetTempFileName();

        if (shortLived)
        { 
            // Set the temporary attribute, meaning the file will live 
            // in memory and will not be written to disk 
            //
            File.SetAttributes(temporaryFile, 
                File.GetAttributes(temporaryFile) | FileAttributes.Temporary);
        }

        return temporaryFile;
    }
}

Using the new class is easy, just type the following:

using (TemporaryFile temporaryFile = new TemporaryFile())
{
    // Use temporary file
}

If you decide, after constructing a TemporaryFile, that you want to prevent it from being deleted, simply set the TemporaryFile.Keep property to true:

using (TemporaryFile temporaryFile = new TemporaryFile()) 
{ 
    temporaryFile.Keep = true; 
}

Absolutely. This way you can ensure cleanup with exceptions present.

You should definitely use Dispose to clean up resources, but make sure you implement the IDisposable interface. You don't want to just add a method named Dispose.

I always make my classes that point to temp files IDisposable, and usually implement a finalizer that calls my dispose method there as well. This seems to be the paradigm suggested by the IDisposable MSDN page.

Related code below:

public void Dispose()
{
    Dispose(true);
    // This object will be cleaned up by the Dispose method.
    // Therefore, you should call GC.SupressFinalize to
    // take this object off the finalization queue
    // and prevent finalization code for this object
    // from executing a second time.
    GC.SuppressFinalize(this);
}

// Dispose(bool disposing) executes in two distinct scenarios.
// If disposing equals true, the method has been called directly
// or indirectly by a user's code. Managed and unmanaged resources
// can be disposed.
// If disposing equals false, the method has been called by the
// runtime from inside the finalizer and you should not reference
// other objects. Only unmanaged resources can be disposed.
private void Dispose(bool disposing)
{
    // Check to see if Dispose has already been called.
    if(!this.disposed)
    {
        // If disposing equals true, dispose all managed
        // and unmanaged resources.
        if(disposing)
        {
            // Dispose managed resources.

        }

        // Call the appropriate methods to clean up
        // unmanaged resources here.
        // If disposing is false,
        // only the following code is executed.


        // Note disposing has been done.
        disposed = true;

    }
}



// Use C# destructor syntax for finalization code.
// This destructor will run only if the Dispose method
// does not get called.
// It gives your base class the opportunity to finalize.
// Do not provide destructors in types derived from this class.
~MyResource()
{
    // Do not re-create Dispose clean-up code here.
    // Calling Dispose(false) is optimal in terms of
    // readability and maintainability.
    Dispose(false);
}

If you wish to re-use your temporary files e.g. open\close\read\write\etc, then clearing them up at the AppDomain unload level can be useful.

This can be used in combination with putting temp files in a well known sub-directory of a temp location and making sure that the directory is deleted on application startup to ensure unclean shut-downs are taken care of.

A basic example of the technique (with exception handling removed around delete for brevity). I use this technique in file-based unit tests where it makes sense and is useful.

public static class TempFileManager
{
    private static readonly List<FileInfo> TempFiles = new List<FileInfo>();
    private static readonly object SyncObj = new object();

    static TempFileManager()
    {
        AppDomain.CurrentDomain.DomainUnload += CurrentDomainDomainUnload;
    }

    private static void CurrentDomainDomainUnload(object sender, EventArgs e)
    {
        TempFiles.FindAll(file => File.Exists(file.FullName)).ForEach(file => file.Delete());
    }

    public static FileInfo CreateTempFile(bool autoDelete)
    {
        FileInfo tempFile = new FileInfo(Path.GetTempFileName());

        if (autoDelete)
        {
            lock (SyncObj)
            {
                TempFiles.Add(tempFile);
            }
        }

        return tempFile;
    }
}