1

I have been reading a lot of articles about Singleton online, but most of the articles out only demonstrate creating simple objects with properly locking to ensure thread safe without race condition.

Simple enough to demonstrate, I put a file logging method in there. It will work for 10-20 concurrent threads but when I try 100 threads, it will fail giving "The process cannot access the file because it is being used by another process".

public class Logger
{

    private string _path = "c:\\temp\\MyTestLog.txt";

    // Singleton with padlock for DCL
    private volatile static Logger _instance;
    private static readonly object  _padLock = new Object();

    // Singleton
    public static Logger Instance()
    {
        // Implement Double Check Locking (MSDN - Thread-safe)
        if (_instance == null)
        {
            lock (_padLock)
            {
                if (_instance == null)
                    _instance = new Logger();
            }
        }

        return _instance;
    }

    // Simple WriteLog 
    public void WriteLog(string text, out bool result)
    {
        try
        {             
            // Write to file 
            using (Stream stream = File.Open(_path, FileMode.Append))
            {
                using (TextWriter writer = new StreamWriter(stream))
                {
                    writer.WriteLine(text);
                }
            }

            Debug.WriteLine(text);
            result = true;
        }
        catch (Exception ex)
        {
            Debug.WriteLine(ex.ToString());
            result = false;
        }
    }

}

To call this from client, I setup a simple Windows Form and Threading to generate multi-threads by calling:

Logger.Instance().WriteLog("test", out result);

Well, I know the reason why it failed because the previous _instance is passed to the next thread where the inner file operation wasn't completed that causes the error. But not sure how to solve this.

I also tried this.. If I replace the File IO writing with SqlConnection, I will get "The connection was not closed. The connection's current state is open" instead of "The process cannot access the file because it is being used by another process". Same idea. Of course you will use Connection Pool instead of Singleton but just for my own curiosity.

I read "C# in Depth" article and tried different ways and didn't work. Even with .NET 4.0 Lazy doesn't solve the problem. The only workaround successfully is break the Singleton rule by using [ThreadStatic] attribute on _instance and DCL is no longer needed. But then it turns the class into a multi-threaded class with lazy initialization, not real Singleton anymore.

So, I am wondering.. how does Singleton solve this problem?

4
  • 4
    The locking mechanism you've given ensures that only one instance of Logger ever exists (assuming it's correct)... but you're doing nothing to stop 100 threads being in WriteLog at once!
    – AakashM
    Feb 13, 2014 at 15:17
  • @AakashM Is right. You're attempting to write to the same file concurrently. SQL would create a new connection, you wouldn't be sharing the same connection there Feb 13, 2014 at 15:20
  • The problem is not in the singleton implementation. The issue is in the WriteLog method: multiple threads can access it and open/write/close the file concurrently. You should consider to reuse the same TextWriter instance and synchronize the access to that method.
    – Alberto
    Feb 13, 2014 at 15:26
  • Also keep in mind that .NET has the concept of application domains. If you decide to put this Logger class into a utility DLL and then call it from other DLLs, then the singleton pattern as-is breaks. We were bit by that until we looked into cross app-domain singletons for a more robust solution for this case.
    – jia103
    Feb 13, 2014 at 15:27

1 Answer 1

6

As stated in the comments, your problem is that WriteLog is not thread safe at all.

Having a singleton does not mean that every single method of that class will be in the same thread.

Even if you only have one instance, its methods can still be called by multiple threads at the same time.

Solution:

Put some synchronization logic in your WriteLog method. Lock statement, ReaderWriterLockSlim... the choice is all yours!

I recommend this article if you want to know more about threading in c#:

http://www.albahari.com/threading/

1
  • Thanks a lot!!! So, basically I added this attribute to the WriteLog method and it seems to solve the problem! You guys are so helpful. [MethodImpl(MethodImplOptions.Synchronized)]
    – userb00
    Feb 13, 2014 at 15:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.