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);
        }
    }
}

If you make the creator 'slow' (let the thread sleep for a while) you increase the chance that other threads will hit the lock and will have to wait for the slow thread to leave the lock. — Emond, Jan 25 '16 at 09:33
You're trying to unit test thread safety? That seems odd to me. It sounds to me like you'd unit test the functionality of the class you expose, where the thread-safety issue would be more of a subject for code reviewing this code. — Yuval Itzchakov, Jan 25 '16 at 09:36
If someone comes and delete the `lock` portion of the code, and the code review didn't catch it, I wan't that the unit test to catch it. — Cyril Gandon, Jan 25 '16 at 09:52
Why not simply use the `Lazy` class? It seems to do exactly what you are trying to accomplish. [link](https://msdn.microsoft.com/en-us/library/dd642331(v=vs.110).aspx) — Maarten, Jan 25 '16 at 09:58
@link: yes `Lazy` resolve the problem on this small example. My real case is more complex and involves a more advanced cases where I can't use the `Lazy` class. — Cyril Gandon, Jan 25 '16 at 13:53
Why not use rhino mock and use expect for method call for once only? — vendettamit, Jan 27 '16 at 05:22

Lucas Trzesniewski · Accepted Answer · 2016-01-28T10:50:41.883

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.

Hi @Lucas, the method looks promising, but the test is OK if I comment the `lock code`, and I start the second thread one second after: `var t2 = Task.Factory.StartNew(() => { Thread.Sleep(1000); return cache.Get(creator); });` — Cyril Gandon, Jan 28 '16 at 10:05
Wonderful, that's exactly what I was looking for, Thread manipulation and Thread state. Thank you. — Cyril Gandon, Jan 28 '16 at 11:07

score 1 · Answer 2 · answered Jan 25 '16 at 09:38

1

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.

answered Jan 25 '16 at 09:38

pid

11,472
6
34
63

What does Yield add here, isn't sleep sufficient? What does the processor have to do with this? – Emond Jan 25 '16 at 10:52
The idea is to unload the running thread so that the other concurrent threads can run and request the lock. Yield allows to abort the current time slice so that the round-robin can swap-in another time slice immediately. – pid Jan 25 '16 at 13:42
Sleep will do the same thing AND across processors – Emond Jan 25 '16 at 13:43
Ah, ok. Good to know! Thanks. – pid Jan 25 '16 at 13:43

score 1 · Answer 3 · answered Jan 28 '16 at 05:46

In addition to pid's answer:
This code doesn't actually create a lot of threads.

// Launch a very big number of thread to be sure
Parallel.ForEach(Enumerable.Range(0, 100), _ =>
{
    cache.Get(creator);
});

It will start ~Environment.ProcessorCount threads. More details.

If you want to get a lot of threads you should do it explicitly.

var threads = Enumerable.Range(0, 100)
    .Select(_ => new Thread(() => cache.Get(creator))).ToList();
threads.ForEach(thread => thread.Start());
threads.ForEach(thread => thread.Join());

So if you will have enough threads and you will enforce them to switch you will get concurrent race.

If you care about case when your CI server will have only one free core, you can include this constraint in your test by changing Process.ProcessorAffinity property. More details.

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

3 Answers3