Managing disposable objects within static methods

Question

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?

score 7 · Accepted Answer · answered Feb 01 '12 at 14:36

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.

So, are you saying the only reason it appeared to work when I changed it to use non-static methods was because I got 'lucky' when testing it? — fearofawhackplanet, Feb 01 '12 at 14:42
@fearofa - Looks like it, you are in a position to test. Put a Sleep(10) inside the static versions. — H H, Feb 01 '12 at 15:16

score 3 · Answer 2 · edited May 23 '17 at 10:34

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);          
        }
    }      
}

Managing disposable objects within static methods

2 Answers2