How to unit test code that should execute only once in a MultiThread scenario?

Question

A class contains an attribute that should be created only one time. The creation process is via a Func<T> which is pass in argument. This is a part of a caching scenario.

The test take care that no matter how many threads try to access the element, the creation occurs only once.

The mechanism of the unit test is to launch a great number of threads around the accessor, and count how many times the creation function is called.

This is not deterministic at all, nothing guaranteed that this is effectively testing a multithread access. Maybe there will be only one thread at a time that will hit the lock. (In reality, getFunctionExecuteCount is between 7 and 9 if the lock is not there... On my machine, nothing guaranteed that on the CI server it's going to be the same)

How the unit test can be rewritten in a deterministic way? How to be sure that the lock is triggered multiple times by multiple thread?

using Microsoft.VisualStudio.TestTools.UnitTesting;
using System;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace Example.Test
{
    public class MyObject<T> where T : class
    {
        private readonly object _lock = new object();
        private T _value = null;
        public T Get(Func<T> creator)
        {
            if (_value == null)
            {
                lock (_lock)
                {
                    if (_value == null)
                    {
                        _value = creator();
                    }
                }
            }
            return _value;
        }
    }

    [TestClass]
    public class UnitTest1
    {
        [TestMethod]
        public void MultipleParallelGetShouldLaunchGetFunctionOnlyOnce()
        {
            int getFunctionExecuteCount = 0;
            var cache = new MyObject<string>();

            Func<string> creator = () =>
            {
                Interlocked.Increment(ref getFunctionExecuteCount);
                return "Hello World!";
            };
            // Launch a very big number of thread to be sure
            Parallel.ForEach(Enumerable.Range(0, 100), _ =>
            {
                cache.Get(creator);
            });

            Assert.AreEqual(1, getFunctionExecuteCount);
        }
    }
}

The worst scenario is if someone break the lock code, and the testing server had some lag. This test shouldn't pass:

using NUnit.Framework;
using System;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace Example.Test
{
    public class MyObject<T> where T : class
    {
        private readonly object _lock = new object();
        private T _value = null;
        public T Get(Func<T> creator)
        {
            if (_value == null)
            {
                // oups, some intern broke the code
                //lock (_lock)
                {
                    if (_value == null)
                    {
                        _value = creator();
                    }
                }
            }
            return _value;
        }
    }

    [TestFixture]
    public class UnitTest1
    {
        [Test]
        public void MultipleParallelGetShouldLaunchGetFunctionOnlyOnce()
        {
            int getFunctionExecuteCount = 0;
            var cache = new MyObject<string>();

            Func<string> creator = () =>
            {
                Interlocked.Increment(ref getFunctionExecuteCount);
                return "Hello World!";
            };
            Parallel.ForEach(Enumerable.Range(0, 2), threadIndex =>
            {
                // testing server has lag
                Thread.Sleep(threadIndex * 1000);
                cache.Get(creator);
            });

             // 1 test passed :'(
            Assert.AreEqual(1, getFunctionExecuteCount);
        }
    }
}

Answer 1

To make it deterministic, you only need two threads and ensure one of them blocks inside the function, while the other tries to get inside as well.

[TestMethod]
public void MultipleParallelGetShouldLaunchGetFunctionOnlyOnce()
{
    var evt = new ManualResetEvent(false);

    int functionExecuteCount = 0;
    var cache = new MyObject<object>();

    Func<object> creator = () =>
    {
        Interlocked.Increment(ref functionExecuteCount);
        evt.WaitOne();
        return new object();
    };

    var t1 = Task.Run(() => cache.Get(creator));
    var t2 = Task.Run(() => cache.Get(creator));

    // Wait for one task to get inside the function
    while (functionExecuteCount == 0)
        Thread.Yield();

    // Allow the function to finish executing
    evt.Set();

    // Wait for completion
    Task.WaitAll(t1, t2);

    Assert.AreEqual(1, functionExecuteCount);
    Assert.AreEqual(t1.Result, t2.Result);
}

You may want to set a timeout on this test :)

---

Here's a variant allowing to test more cases:

public void MultipleParallelGetShouldLaunchGetFunctionOnlyOnce()
{
    var evt = new ManualResetEvent(false);

    int functionExecuteCount = 0;
    var cache = new MyObject<object>();

    Func<object> creator = () =>
    {
        Interlocked.Increment(ref functionExecuteCount);
        evt.WaitOne();
        return new object();
    };

    object r1 = null, r2 = null;

    var t1 = new Thread(() => { r1 = cache.Get(creator); });
    t1.Start();

    var t2 = new Thread(() => { r2 = cache.Get(creator); });
    t2.Start();

    // Make sure both threads are blocked
    while (t1.ThreadState != ThreadState.WaitSleepJoin)
        Thread.Yield();

    while (t2.ThreadState != ThreadState.WaitSleepJoin)
        Thread.Yield();

    // Let them continue
    evt.Set();

    // Wait for completion
    t1.Join();
    t2.Join();

    Assert.AreEqual(1, functionExecuteCount);
    Assert.IsNotNull(r1);
    Assert.AreEqual(r1, r2);
}

If you want to delay the second call, you won't be able to use Thread.Sleep, as it'll cause the thread to go to the WaitSleepJoin state:

The thread is blocked. This could be the result of calling Thread.Sleep or Thread.Join, of requesting a lock — for example, by calling Monitor.Enter or Monitor.Wait — or of waiting on a thread synchronization object such as ManualResetEvent.

And we won't be able to tell if the thread is sleeping or waiting on your ManualResetEvent...

But you can easily substitute the sleep with a busy wait. Comment out the lock, and change t2 to:

var t2 = new Thread(() =>
{
    var sw = Stopwatch.StartNew();
    while (sw.ElapsedMilliseconds < 1000)
        Thread.Yield();

    r2 = cache.Get(creator);
});

Now the test will fail.

Answer 2

I don't think a really deterministic way exists, but you can raise the probability so that it's very difficult to not cause concurrent races:

Interlocked.Increment(ref getFunctionExecuteCount);
Thread.Yield();
Thread.Sleep(1);
Thread.Yield();
return "Hello World!";

By raising the Sleep() parameter (to 10?) it gets more and more improbable that no concurrent race takes place.