CODEWITHSUNDEEP
c#.netstaticstreamdisposable
fearofawhackplanet
fearofawhackplanet

Reputation: 53446

Managing disposable objects within static methods

public class SimpleLogger
{
    static readonly string logFile = ConfigurationManager.AppSettings["LogFile"];

    static StreamWriter GetStream()
    {
        return File.Exists(logFile) ?
            File.AppendText(logFile) : File.CreateText(logFile);
    }

    public static void Write(string msg)
    {
        using (var sw = GetStream())
        {
            sw.Write(msg);
        }
    }
}

The above code fails in use as it doesn't appear to be closing/disposing of the stream correctly. Subsequent writes give a 'file in use' IOException.

If the class is modified to use non-static methods, it appears to work correctly.

I don't understand why there would be any behavioural difference?

Upvotes: 3

Views: 1449

Answers (2)

Jeb
Jeb

Reputation: 3799

I don't think changing from a static to an instance would fix the problem since they're all ultimately contending over a static resource (the file). This answer might help you. Perhaps if you left both methods static and declared a static synchronisation object for calling threads to lock with (since the resource is static itself) would help?, e.g.:

private static object _objectLock = new object();

for synchronising access to the file from multiple threads, hence:

public static void Write(string msg)      
{        
    lock(_objectLock)
    {  
        using (var sw = GetStream())          
        {              
            sw.Write(msg);          
        }
    }      
}

Upvotes: 3

Marc Gravell
Marc Gravell

Reputation: 1064004

The disposal is fine; GetStream delivers an open writer; Write closes/disposes it - sorted. if I had to guess, though, the issue is concurrent use - i.e. multiple threads (in particular in a web application) accessing the file at the same time. If that is the case, options:

  • make Write (and any other access to the file) synchronized, so only one caller can possibly try to have the file open at once
  • use a pre-canned logging framework that will already handle this scenario (common approaches here include synchronization, but also: buffering the data locally and then pushing the data down periodically - avoids opening the file over and over and over and over)

In particular; your only static state is the file path itself. There will therefore be no significant difference between using this as a static versus instance method.

As a side-note, File.AppendAllText may be useful here, but does not avoid the issue of concurrency.

Upvotes: 7

Related Questions