0

So, I don't have a lot of experience dealing with concurrent problems so I've decided to ask a question, regarding a piece of code.

I have class:

public class SimpleClass
{
    public int OwnNumber { get; set; }
    public int ActualNumber { get; set; }
}

there are two extension methods working with this class:

public static class SimpleClassHelper
{
    public static int Apply(this SimpleClass sc)
    {
        int z = 1;
        for (var i = 0; i < 1000; i++)
        {
            z += i;
        }
        return sc.ActualNumber;
    }

    public static IEnumerable<SimpleClass> Test(this IEnumerable<SimpleClass> sc) =>
        sc.AsParallel().Select(s => new SimpleClass
        {
            OwnNumber = s.OwnNumber,
            ActualNumber = s.Apply()
        });
}

and in my Program.cs I have this:

    static void Main(string[] args)
    {
        var s = new SimpleClass { OwnNumber = 0, ActualNumber = 0 };
        var s1 = new SimpleClass { OwnNumber = 1, ActualNumber = 1 };
        var s2 = new SimpleClass { OwnNumber = 2, ActualNumber = 2 };
        var s3 = new SimpleClass { OwnNumber = 3, ActualNumber = 3 };
        var s4 = new SimpleClass { OwnNumber = 4, ActualNumber = 4 };
        var s5 = new SimpleClass { OwnNumber = 5, ActualNumber = 5 };
        var s6 = new SimpleClass { OwnNumber = 6, ActualNumber = 6 };
        var s7 = new SimpleClass { OwnNumber = 7, ActualNumber = 7 };

        List<SimpleClass> seq = new List<SimpleClass>();
        seq.Add(s);
        seq.Add(s1);
        seq.Add(s2);
        seq.Add(s3);
        seq.Add(s4);
        seq.Add(s5);
        seq.Add(s6);
        seq.Add(s7);

        for (var i = 0; i < 10; i++)
        {
            var res = seq.Test();
            foreach (var item in res)
            {
                Console.WriteLine($"{item.OwnNumber} : {item.ActualNumber}");
            }
        }
    }

The code is a bit too much but it's a fully working example and I think, it quite well displays my confusion.

My train of thought is this:

  • I create a collection seq
  • I call 10 times Test() on this collection
  • Each time I call Test the collection is separated into several chunks executed in parallel
  • At some point when I apply the value from s.Apply() I expect that at least some percentage of the time the reference in the Apply() method will be replaced with reference with from some of the parallel threads and I will get different OwnNumber and ActualNumber.

However I made several runs and everything seems to be in order but with concurrency you never know. Is this an expected behavior of AsParalle()? Since all objects are calling the same Apply() method why I never see the behavior described above? If, say, 3 threads are calling Apply() at the same time I expect at least at several points to get the value from some other object especially since I have some delay with the for loop?

Well, it seems that maybe I'm wrong in my assumptions but then is there something preveting this code from race conditions?

Leron
  • 9,546
  • 35
  • 156
  • 257
  • 1
    You know that you are never using `z` in the `Apply` method? – Rand Random Apr 10 '20 at 12:16
  • Yes, maybe completely useless piece of code, just something to replace thread sleep. I wanted ti simulate doing some work before returning the value. – Leron Apr 10 '20 at 12:23
  • @Leron_says_get_back_Monica It's better to put `Thread.Sleep` or `await Task.Delay`, because such piece of code is executed immediately :) – Michał Turczyn Apr 10 '20 at 12:30
  • Is your question centered around the "extensiveness" of the `Apply` method? In other words, if the `Apply` was a normal instance member of the `SimpleClass`, you would have no thread-safety concerns in that case? – Theodor Zoulias Apr 10 '20 at 12:31
  • @TheodorZoulias my main confusion is that `Apply` is a static method which works on a reference type. **Random Random** answer makes a lot of sense but there still a bit of confusion how a static method which in theroy (at least I think) can be called from different threads, guarantees that if you call the method with one object, the object will remain the same till the end of the method. In concurrent programming, usually static types have exactly the opposite purpose, to share data between threads, not to encapsulate. – Leron Apr 10 '20 at 12:46
  • Lets consider the [`Math.Min`](https://learn.microsoft.com/en-us/dotnet/api/system.math.min) method. It is a static method that accepts two arguments. Are you concerned that if this method is called by multiple threads in parallel, the arguments of one invocation may be somehow interchanged with the arguments of another invocation? – Theodor Zoulias Apr 10 '20 at 13:02
  • 1
    Although not directly related to this question, you should be aware that every method (static or not) can be inlined by the compiler, and this could have thread-safety implications. Take a look at this question for an example: [Is Extension method thread safe?](https://stackoverflow.com/questions/4618872/is-extension-method-thread-safe) – Theodor Zoulias Apr 10 '20 at 13:06
  • Btw you could consider adding the tag #thread-safety to your question. It sounds relevant. – Theodor Zoulias Apr 10 '20 at 13:13

1 Answers1

1

After giving it some thought, I believe to know what your confusion is.

Let's have a look at your Test method?

public static IEnumerable<SimpleClass> Test(this IEnumerable<SimpleClass> sc) =>
    sc.AsParallel().Select(s => new SimpleClass
    {
        OwnNumber = s.OwnNumber,
        ActualNumber = s.Apply()
    });

.Select(s => {}) <- this will "tranform" each local variable s

And that makes your current code thread safe without any concurrence problems, since s is a local variable, there is no way that a different thread can change the reference of a local variable.

To make your code fail, you would need to have a shared variable that more threads could access at the same time at any given point of time.

For example if your code looked like this:

public static class SimpleClassHelper
{
    private static SimpleClass _sharedSimpleClassHelper;

    public static int Apply()
    {
        int z = 1;
        for (var i = 0; i < 1000; i++)
        {
            z += i;
        }

        return _sharedSimpleClassHelper.ActualNumber; //use the shared variable
    }

    public static IEnumerable<SimpleClass> Test(this IEnumerable<SimpleClass> sc) =>
        sc.AsParallel().Select(s => 
        {
            _sharedSimpleClassHelper = s; //pass the local variable to a shared variable, that can be referenced from multiple threads
            return new SimpleClass
            {
                OwnNumber = s.OwnNumber,
                ActualNumber = Apply()
            };
        });
}

You can see it in action here: https://dotnetfiddle.net/rF68e2


Edit:

Based on your comment

but there still a bit of confusion how a static method which in theroy (at least I think) can be called from different threads, guarantees that if you call the method with one object, the object will remain the same till the end of the method. In concurrent programming, usually static types have exactly the opposite purpose, to share data between threads, not to encapsulate.

Yes, a static method, like any method can be called from different threads at the same time, but what decides if a method is thread-safe isn't if the method is declared static or not, it is if threads share a state or not.

Let's again look at your static apply method (removed the for loop, to make it easier to look at)

public static int Apply(this SimpleClass sc)
{
    return sc.ActualNumber;
}

What makes you believe that the referenc sc could be modified by anything?

sc is a local variable that only has the scope of this method, nothing outside of this method can give this local variable a different reference.

So even if Apply gets called x time at the same time, each time all those local variable will have a pointer to the passed in class.

Of course if for exmple your method looked like this:

public static int Apply(this SimpleClass sc)
{
    var localVariable_ActualNumber = sc.ActualNumber;
    localVariable_ActualNumber += 1;
    return localVariable_ActualNumber;
}

This code isn't thread safe since all thread are accessing and manipulating an shared memory the ActualNumber.

For exmple:

  1. Thread 1 reads ActualNumber (1)
  2. Thread 2 reads ActualNumber (1)
  3. Thread 1 += 1
  4. Thread 1 writes ActualNumer (2)
  5. Thread 3 reads ActualNumber (2)
  6. Thread 3 += 1
  7. Thread 3 writes ActualNumer (3)
  8. Thread 2 += 1
  9. Thread 2 writes ActualNumer (2)

Vs No Threads

  1. reads ActualNumber (1)
  2. += 1
  3. writes ActualNumer (2)
  4. reads ActualNumber (2)
  5. += 1
  6. writes ActualNumer (3)
  7. reads ActualNumber (3)
  8. += 1
  9. writes ActualNumer (4)

So even though the method got entered 3 times the ActualNumber will only be 2 instead of 4.

But you may ask, why do the theads now share memory?

Because the local variable is only a pointer to a single SimpleClass, if you access and modify a value inside of the SimpleClass it is valid for all threads.

You could also ask yourself do you believe that any thread can accees localVariable_ActualNumber?

No, they can't.

The same applies to sc, a local variable can't be access by different thread.

But if you where to manipulate sc.ActualNumber this can cause thread-safe issues, since each sc points to the same object in memory and ActualNumber is shared accross threads.

Rand Random
  • 7,300
  • 10
  • 40
  • 88