CODEWITHSUNDEEP
c#multithreadingfile-io
Firoso
Firoso

Reputation: 6685

Multi-threaded file write enqueueing

So I have a static class that is supposed to be used as a log file manager, capable of adding "messages" (strings) to a Queue object, and that will push messages out to a file. Trouble is, many different threads should be enqueueing, and that the writer needs to be async as well. Currently when I insert into the queue, I'm also checking to see if the writer is writing (bool check), if it's not, i set the bool and then start the writing, but I'm getting intermittent IO exceptions about file access, and then wierd writing behavior sometimes.

Someone want to give me a hand on this?

Upvotes: 2

Views: 8635

Answers (5)

Ryan Fernandes
Ryan Fernandes

Reputation: 8526

Let me address the problem at a different level:

If your writing a business application then you'd want to focus on the business-logic portions rather than the infrastructural code, more so if this infra code is already available, tested and deployed to multiple production sites (taking care of your NFRs)

I'm sure you're aware of the existance of logging frameworks like log4net and others http://csharp-source.net/open-source/logging.

Have you given these a chance before hand-rolling out your own Logger?

Take this option to the technical architect of the enterprise you're writing for and see she thinks.

Cheers

Upvotes: 1

Jeff Sternal
Jeff Sternal

Reputation: 48583

If you don't want to restructure your code dramatically like I suggested in my other answer, you could try this, which assumes your LogManager class has:

  • a static thread-safe queue, _SynchronizedQueue
  • a static object to lock on when writing, _WriteLock

and these methods:

public static void Log(string message) {
    LogManager._SynchronizedQueue.Enqueue(message);
    ThreadPool.QueueUserWorkItem(LogManager.Write(null));
}

// QueueUserWorkItem accepts a WaitCallback that requires an object parameter
private static void Write(object data) {
    // This ensures only one thread can write at a time, but it's dangerous
    lock(LogManager._WriteLock) {
        string message = (string)LogManager._SynchronizedQueue.Dequeue();
        if (message != null) {
            // Your file writing logic here
        }
    }
}

There's only one problem: the lock statement in the Write method above will guarantee only one thread can write at a time, but this is dangerous. A lot can go wrong when trying to write to a file, and you don't want to hold onto (block) thread pool threads indefinitely. Therefore, you need to use a synchronization object that lets you specify a timeout, such as a Monitor, and rewrite your Write method like this:

private static void Write() {
    if (!Monitor.TryEnter(LogManager._WriteLock, 2000)) {
       // Do whatever you want when you can't get a lock in time
    } else {
      try {
         string message = (string)LogManager._SynchronizedQueue.Dequeue();
         if (message != null) {
             // Your file writing logic here
         }
      }
      finally {
        Monitor.Exit(LogManager._WriteLock);
      }
    }
}

Upvotes: 3

Jeff Sternal
Jeff Sternal

Reputation: 48583

It sounds like the queue is driving the file writing operation. I recommend that you invert the control relationship so that the writer drives the process and checks the queue for work instead.

The simplest way to implement this is to add a polling mechanism to the writer in which it checks the queue for work at regular intervals.

Alternately, you could create an observerable queue class that notifies subscribers (the writer) whenever the queue transitions from empty: the subscribing writer could then begin its work. (At this time, the writer should also unsubscribe from the queue's broadcast, or otherwise change the way it reacts to the queue's alerts.)

After completing its job, the writer then checks the queue for more work. If there's no more work to do, it goes to sleep and resume polling or goes to sleep and resubscribes to the queue's alerts.

As Irwin noted in his answer, you also need to use the thread-safe wrapper provided by the Queue class' Synchronized method or manually synchronize access to your Queue if multiple threads are reading from it and writing to it (as in SpaceghostAli's example).

Upvotes: 2

HasaniH
HasaniH

Reputation: 8402

You should synchronize around your queue. Have multiple threads send to the queue and a single thread read from the queue and write to the file.

public void Log(string entry)
{
    _logMutex.WaitOne();
    _logQueue.Enqueue(entry);
    _logMutex.ReleaseMutex();
}

private void Write()
{
    ...
    _logMutex.WaitOne();
    string entry = _logQueue.Dequeue();
    _logMutex.ReleaseMutex();

    _filestream.WriteLine(entry);
    ...
}

Upvotes: 2

Irwin
Irwin

Reputation: 12819

I would have just one thread doing the writes to avoid contentions, while i would use multiple threads to enqueue.

You are advised "To guarantee the thread safety of the Queue, all operations must be done through the wrapper returned by the Synchronized method." - from http://msdn.microsoft.com/en-us/library/system.collections.queue.aspx

Upvotes: 2

Related Questions