2

I've been trying to solve this issue for quite some time now. I've written some example code showcasing the usage of lock in C#. Running my code manually I can see that it works the way it should, but of course I would like to write a unit test that confirms my code.

I have the following ObjectStack.cs class:

enum ExitCode
{
    Success = 0,
    Error = 1
}

public class ObjectStack
{
    private readonly Stack<Object> _objects = new Stack<object>();
    private readonly Object _lockObject = new Object();
    private const int NumOfPopIterations = 1000;

    public ObjectStack(IEnumerable<object> objects)
    {
        foreach (var anObject in objects) {
            Push(anObject);
        }
    }

    public void Push(object anObject)
    {
        _objects.Push(anObject);
    }

    public void Pop()
    {
        _objects.Pop();
    }

    public void ThreadSafeMultiPop()
    {
        for (var i = 0; i < NumOfPopIterations; i++) {
            lock (_lockObject) {
                try {
                    Pop();
                }
                //Because of lock, the stack will be emptied safely and no exception is ever caught
                catch (InvalidOperationException) {
                    Environment.Exit((int)ExitCode.Error);
                }
                if (_objects.Count == 0) {
                    Environment.Exit((int)ExitCode.Success);
                }
            }
        }
    }

    public void ThreadUnsafeMultiPop()
    {
        for (var i = 0; i < NumOfPopIterations; i++) {
                try {
                    Pop();
                }
                //Because there is no lock, an exception is caught when popping an already empty stack
                catch (InvalidOperationException) {
                    Environment.Exit((int)ExitCode.Error);
                }
                if (_objects.Count == 0) {
                    Environment.Exit((int)ExitCode.Success);
                }
            }
    }
}

And Program.cs:

public class Program
{
    private const int NumOfObjects = 100;
    private const int NumOfThreads = 10000;

    public static void Main(string[] args)
    {
        var objects = new List<Object>();
        for (var i = 0; i < NumOfObjects; i++) {
            objects.Add(new object());
        }
        var objectStack = new ObjectStack(objects);
        Parallel.For(0, NumOfThreads, x => objectStack.ThreadUnsafeMultiPop());
    }
}

I'm trying to write a unit that tests the thread unsafe method, by checking the exit code value (0 = success, 1 = error) of the executable.

I tried to start and run the application executable as a process in my test, a couple of 100 times, and checked the exit code value each time in the test. Unfortunately, it was 0 every single time.

Any ideas are greatly appreciated!

Sirar Salih
  • 2,514
  • 2
  • 19
  • 18
  • Maybe each thread is emptying their own stacks without any communication? Maybe parallel for is doing the synchronization already? – huseyin tugrul buyukisik Jan 17 '15 at 10:38
  • 1
    Out of interest, why not just use a [`ConcurrentStack`](http://msdn.microsoft.com/en-us/library/dd267331(v=vs.110).aspx) ? – StuartLC Jan 17 '15 at 10:39
  • Your test is not correct. In `Main` you only create 100 of objects in `for` loop, and then you are trying to pop out 100 items with 10000 threads? They all emptied by first thread alone. – zmechanic Jan 17 '15 at 11:42
  • `ConcurrentStack` is not used because the point here is to illustrate the usage of `lock` in concurrent code. – Sirar Salih Jan 17 '15 at 16:19

3 Answers3

5

Logically, there is one, very small, piece of code where this problem can happen. Once one of the threads enters the block of code that pops a single element, then either the pop will work in which case the next line of code in that thread will Exit with success OR the pop will fail in which case the next line of code will catch the exception and Exit.

This means that no matter how much parallelization you put into the program, there is still only one single point in the whole program execution stack where the issue can occur and that is directly before the program exits.

The code is genuinely unsafe, but the probability of an issue happening in any single execution of the code is extremely low as it requires the scheduler to decide not to execute the line of code that will exit the environment cleanly and instead let one of the other Threads raise an exception and exit with an error.

It is extremely difficult to "prove" that a concurrency bug exists, except for really obvious ones, because you are completely dependent on what the scheduler decides to do.

Looking up some other posts I see this post which is written related to Java but references C#: How should I unit test threaded code?

It includes a link to this which might be useful to you: http://research.microsoft.com/en-us/projects/chess/

Hope this is useful and apologies if it is not. Testing concurrency is inherently unpredictable as is writing example code to cause it.

Community
  • 1
  • 1
Mike Curry
  • 1,609
  • 1
  • 9
  • 12
  • That's nice somebody remembers the `chess` research project. It was just as interesting as `pex` - completely offtopic, but also worth checking out! – quetzalcoatl Jan 17 '15 at 12:42
1

Thanks for all the input! Although I do agree that this is a concurrency issue quite hard to detect due to the scheduler execution among other things, I seem to have found an acceptable solution to my problem.

I wrote the following unit test:

[TestMethod]
public void Executable_Process_Is_Thread_Safe()
{
   const string executablePath = "Thread.Locking.exe";
        for (var i = 0; i < 1000; i++) {
            var process = new Process() {StartInfo = {FileName = executablePath}};
            process.Start();
            process.WaitForExit();
            if (process.ExitCode == 1) {
                Assert.Fail();
            }
        }
}

When I ran the unit test, it seemed that the Parallel.For execution in Program.cs threw strange exceptions at times, so I had to change that to traditional for-loops:

public class Program
{
    private const int NumOfObjects = 100;
    private const int NumOfThreads = 10000;

    public static void Main(string[] args)
    {
        var objects = new List<Object>();
        for (var i = 0; i < NumOfObjects; i++) {
            objects.Add(new object());
        }

        var tasks = new Task[NumOfThreads];
        var objectStack = new ObjectStack(objects);
        for (var i = 0; i < NumOfThreads; i++)
        {
            var task = new Task(objectStack.ThreadUnsafeMultiPop);
            tasks[i] = task;
        }
        for (var i = 0; i < NumOfThreads; i++)
        {
            tasks[i].Start();
        }

        //Using this seems to throw exceptions from unit test context
        //Parallel.For(0, NumOfThreads, x => objectStack.ThreadUnsafeMultiPop());
}

Of course, the unit test is quite dependent on the machine you're running it on (a fast processor may be able to empty the stack and exit safely before reaching the critical section in all cases).

Sirar Salih
  • 2,514
  • 2
  • 19
  • 18
0

1.) You could inject IL Inject Context switches on a post build of your code in the form of Thread.Sleep(0) using ILGenerator which would most likely help these issues to arise.

2.) I would recommend you take a look at the CHESS project by Microsoft research team.

Martin54
  • 1,349
  • 2
  • 13
  • 34
Roman Ambinder
  • 369
  • 3
  • 7