HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

File Async writer using Tasks or new features of .Net 4.5?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filenewasyncwritertasksnetusingfeatures

Problem

This code was posted as answer to Async file writer in .Net 3.5.

How would it be done better using Tasks or new features in .Net 4.5?

public sealed class Logger : IDisposable
{
    private delegate void WriteMessage(string message);
    private static readonly Logger Instance = new Logger();
    private static object Locker = new object();
    private static readonly StreamWriter Writer =
                        new StreamWriter("c:\\temp\\logtester.log", true);
    private static bool Disposed;

    private Logger()
    {
    }

    ~Logger()
    {
        Dispose(false);
    }

    public static void Log(string message)
    {
        WriteMessage action = Instance.MessageWriter;
        action.BeginInvoke(message, MessageWriteComplete, action);
    }

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

    private static void Dispose(bool disposing)
    {
        lock (Locker)
        {
            if (Disposed)
            {
                return;
            }

            if (disposing)
            {
                if (Writer != null)
                {
                    Writer.Dispose();
                }
            }

            Disposed = true;
        }
    }

    private static void MessageWriteComplete(IAsyncResult iar)
    {
        ((WriteMessage)iar.AsyncState).EndInvoke(iar);
    }

    private void MessageWriter(string message)
    {
        lock (Locker)
        {
            Writer.WriteLine(message);
        }
    }
}

Solution

Before I explain how would I improve that code, I'll first mention two problems I will not try to fix (because I'm not sure how to do it properly):

-
Disposable singleton is a really weird combination. If something should be available anytime and anywhere, it should be a singleton. On the other hand, if something has a limited lifetime and holds an expensive resource (like an open file), it should be disposable. Those two don't go together very well.

-
Finalizer should execute as fast as possible. It certainly shouldn't take a lock that may not be released for quite some time (because there may be many threads also holding the lock).

The problems my solution will solve:

  • Writing to the log may not be in the correct order, lock doesn't guarantee any ordering.



  • Dispose() may not write all outstanding messages. This is again because lock doesn't guarantee ordering.



  • If a write attempt happens after Dispose() (either because Log() was called after Dispose(), or because of the ordering problems mentioned above), an exception will be thrown on the ThreadPool. This exception can't be caught from outside of Logger and will bring down the whole application. (More reason not to have disposable singletons.)



  • If there are many calls to Log() at the same time, or if the writing takes too long, there may be many threads, all waiting on the same lock.



  • While the message is being written, there is a thread blocking until it finishes.



My solution using TPL Dataflow (which uses .Net 4.5 extensively, available from NuGet) solves the issues so that:

  • Writing to the log will be always in the correct order.



  • Dispose() will first write all outstanding messages before returning.



  • If Log() is called after Dispose(), it will directly throw an exception.



  • There will be at most one thread doing the writing, at any time.



  • While the message is being written, no thread is waiting for it to finish.



public sealed class Logger : IDisposable
{
    private static readonly Logger Instance = new Logger();

    private readonly ActionBlock m_writerBlock;

    private static bool Disposed;

    private Logger()
    {
        var writer = new StreamWriter("c:\\temp\\logtester.log", true);
        m_writerBlock = new ActionBlock(s => writer.WriteLineAsync(s));
        m_writerBlock.Completion.ContinueWith(_ => writer.Dispose());
    }

    public static void Log(string message)
    {
        if (!Instance.m_writerBlock.Post(message))
            throw new ObjectDisposedException("Logger");
    }

    public void Dispose()
    {
        if (Disposed)
            return;

        m_writerBlock.Complete();
        m_writerBlock.Completion.Wait();

        Disposed = true;
    }
}


This works correctly, because the ActionBlock will take care of almost anything needed here, like ordering, synchronization and scheduling Tasks when necessary. WriteLineAsync() returns a Task and the block will use this Task to asynchronously wait for it to finish.

Code Snippets

public sealed class Logger : IDisposable
{
    private static readonly Logger Instance = new Logger();

    private readonly ActionBlock<string> m_writerBlock;

    private static bool Disposed;

    private Logger()
    {
        var writer = new StreamWriter("c:\\temp\\logtester.log", true);
        m_writerBlock = new ActionBlock<string>(s => writer.WriteLineAsync(s));
        m_writerBlock.Completion.ContinueWith(_ => writer.Dispose());
    }

    public static void Log(string message)
    {
        if (!Instance.m_writerBlock.Post(message))
            throw new ObjectDisposedException("Logger");
    }

    public void Dispose()
    {
        if (Disposed)
            return;

        m_writerBlock.Complete();
        m_writerBlock.Completion.Wait();

        Disposed = true;
    }
}

Context

StackExchange Code Review Q#13263, answer score: 13

Revisions (0)

No revisions yet.