TPL Tasks and ReaderWriterLockSlim

Question

I've got a strange bug in my current project. Thats why I have two questions:

1) Why does this occur?
2) How do I solve this?

Some additional info: I'm running this on a System with a QuadCore CPU (Intel Core2Quad Q9650)

I have a Function "DoSomething()" that could be called from multiple Threads at a time:

public class SingletonClass    
{

    // Singleton
    public static SingletonClass Instance 
    {
        get { return _instance ?? (_instance = new SingletonClass()); }
    }

    private readonly ReaderWriterLockSlim _workLock = new ReaderWriterLockSlim();
    private bool _isWorkDone = false;

    // bool returns true for "Inner Task executed" and false for "Inner Task had been executed earlier"
    public bool DoSomething()
    {
        // workaround to fix: Thread.Sleep(50);
        _workLock.EnterWriteLock();
        try
        {
            if (!_isWorkDone)
            {
                Task.Factory.StartNew(DoWork());
                _isWorkDone = true;
                return true;
            }

            return false;
        }
        finally
        {
           _workLock.ExitWriteLock();
        }
    }
}

In order to test if this function works I used the TPL to create multiple Tasks calling this function nearly simultaniously:

for (int i = 0; i < 10; i += 1)
{
    Task.Factory.StartNew(() =>
    {
        bool success = DoSomething();
        Console.WriteLine(success);
    });
}

I expected to get an output like this:

true, false, false, ....

But what I got was this:

true, true, true, true, false, false ....

Remember:
1) Why?
2) How to solve?

EDIT: I added a sample-project: [DELETED - Problem solved]

there is (of course) an other function using EnterReadLock. But that's not the point. I tried to use lock(_lockObject) instead but got the same unexpected output. — myst3rium, Aug 31 '14 at 13:35
May I ask, what CPU you have? If I add Thread.Sleep(50) before `_workLock.EnterWriteLock();` it worked for me. But this is a workaround at most. — myst3rium, Aug 31 '14 at 14:06
Can you add `volatile bool _isWorkDone`? I think `volatile` in C# means **atomic**. — firda, Aug 31 '14 at 14:12
Thank you for your Response! But there was no effect. 4 Threads entering simultaniously. — myst3rium, Aug 31 '14 at 14:13
Than it may be time for Interlocked. I don't think the problem is in the lock, but in the cache - the other thread just don't see it has changed. — firda, Aug 31 '14 at 14:16
I made a small test-program, too and there the problem disappeared ... I'll keep you updated ... — myst3rium, Aug 31 '14 at 14:17
You may try this: http://msdn.microsoft.com/en-us/library/system.threading.interlocked.memorybarrier%28v=vs.110%29.aspx ...but the lock itself should be a barrier, I do not understand. — firda, Aug 31 '14 at 14:18
I reproduced this bug: you remember `bool success = DoSomething();` ? `DoSomething()` is a public function of a Singleton-Class in a DLL referenced by the ConsoleApplication ... (you might to run it multiple times) — myst3rium, Aug 31 '14 at 14:23
This thread says an explicit memory barrier is not needed with ReaderWriterLockSlim - http://stackoverflow.com/questions/20564489/do-i-need-memorybarrier-with-readerwriterlockslim. Strange... — dbc, Aug 31 '14 at 14:27
You should of said it was a Singleton method. Always post a full reproduce. You should edit the code and add the class encapsulating the method — Yuval Itzchakov, Aug 31 '14 at 14:33

score 0 · Accepted Answer · edited Aug 31 '14 at 15:17

No way, the problem is not in the lock but in the creation of the Singleton!

public static Dll Instance
{
    get { return _INSTANCE ?? (_INSTANCE = new Dll()); }
}

The above is not protected by the lock => you get 4 different instances! Four Different locks!

private void CreateThreadsToAccess()
{
    Task[] tasks = new Task[10];
    for (int i = 0; i < tasks.Length; i += 1)
    {
        tasks[i] = Task.Factory.StartNew(() =>
        {
        //  here you get four different instances
            bool success = Dll.Instance.DoSomething();
            Console.WriteLine(success);
        });
    }

    Task.WaitAll(tasks);

    Console.WriteLine("Press any key to exit");
    Console.ReadKey();
}

How to correct:

private static readonly object mylock = new object();
public static Dll Instance
{
    get 
    {
       if (_INSTANCE == null) 
       {
           lock (mylock) 
           {
              if (_INSTANCE == null) 
              {
                 _INSTANCE = new Dll();
              }
           }
       }
       return _INSTANCE;
    }
}

BTW: Look at this question: http://stackoverflow.com/questions/12316406/thread-safe-c-sharp-singleton-pattern — firda, Aug 31 '14 at 14:50

TPL Tasks and ReaderWriterLockSlim

1 Answers1