views:

139

answers:

3

I am making a simple class that contains a StreamWrite

    class Logger
{
    private StreamWriter sw;
    private DateTime LastTime; 
    public Logger(string filename)
    {
        LastTime = DateTime.Now;
        sw = new StreamWriter(filename);
    }
    public void Write(string s)
    {
        sw.WriteLine((DateTime.Now-LastTime).Ticks/10000+":"+ s);
        LastTime = DateTime.Now;
    }
    public void Flush()
    {
        sw.Flush();
    }
    ~Logger()
    {
        sw.Close();//Raises Exception!
    }
}

But when I close this StreamWriter in the destructor, it raises an exception that the StreamWriter was already deleted?

Why? And how to make it work such that when the Logger class is deleted, the StreamWriter is closed before deletion?

Thanks!

+4  A: 

Destructors (a.k.a. Finalizers) are not guaranteed to run in a particular order. See the documentation:

The finalizers of two objects are not guaranteed to run in any specific order, even if one object refers to the other. That is, if Object A has a reference to Object B and both have finalizers, Object B might have already finalized when the finalizer of Object A starts.

Implement IDisposable instead.

oefe
+2  A: 

As a general rule, when it comes to implementing Finalizers, don't. They're typically only required if your class is directly using unmanaged memory. If you do implement a Finalizer, it should never reference any managed members of the class, as they may no longer be valid references.

As an additional caveat, be aware that the Finalizer runs in its own thread, which can trip you up if you're using unmanaged APIs that have thread-affinity. These scenarios are much cleaner with IDisposable and nice, orderly cleanup.

Dan Bryant
+4  A: 

Writing your own destructor (aka finalizer) is wrong in 99.99% of all cases. They are needed to ensure that your class releases an operating system resource that isn't automatically managed by the .NET framework and wasn't properly released by the user of your class.

This starts by having to allocate the operating system resource first in your own code. That always requires some kind of P/Invoke. This is very rarely needed, it was the job of the .NET programmers working at Microsoft to take care of that.

They did in the case of StreamWriter. Through several layers, it is a wrapper around a file handle, created with CreateFile(). The class that created the handle is also the one that's responsible for writing the finalizer. Microsoft code, not yours.

Such a class always implements IDisposable, giving the user of the class an opportunity to release the resource when it is done with it, rather than waiting for the finalizer to get the job done. StreamWriter implements IDisposable.

Of course, your StreamWriter object is a private implementation detail of your class. You don't know when the user is done with your Logger class, you cannot call StreamWriter.Dispose() automatically. You need help.

Get that help by implementing IDisposable yourself. The user of your class can now call Dispose or use the using statement, just like she would with any of the framework classes:

  class Logger : IDisposable {
    private StreamWriter sw;
    public void Dispose() {
      sw.Dispose();   // Or sw.Close(), same thing
    }
    // etc...
  }

Well, that's what you should do in 99.9% of all cases. But not here. At the risk of fatally confusing you: if you implement IDisposable, there must also be a reasonable opportunity for the user of your class to call its Dispose() method. That is normally not much of a problem, except for a Logger type class. It is pretty likely that your user wants to log something up to the last possible moment. So that she can log an unhandled exception from AppDomain.UnhandledException for example.

When to call Dispose() in this case? You could do it when your program terminates. But that's kinda besides the point, there isn't much point in releasing resources early if it happens when the program is exiting.

A logger has the special requirement to shut down properly in all cases. That requires that you set the StreamWriter.AutoFlush property to true so that logging output is flushed as soon as it is written. Given the difficulty of calling Dispose() properly, it is now actually better that you don't implement IDisposable and let Microsoft's finalizer close the file handle.

Fwiw, there's another class in the framework that has the same problem. The Thread class uses four operating system handles but doesn't implement IDisposable. Calling Thread.Dispose() is really awkward, so Microsoft didn't implement it. This causes problems in very unusual cases, you'd need to write a program that creates a lot of threads but never uses the new operator to create class objects. It's been done.

Last but not least: writing a logger is fairly tricky as explained above. Log4net is a popular solution.

Hans Passant