CODEWITHSUNDEEP
c#multithreadingclosures
user2896152
user2896152

Reputation: 816

Propagate a callback onto a separate thread

I've a question about multithreading in C#.

Let's suppose we have this interface

public interface IMyCallback
{
    void OnSum(int a, int b, int result);
}

Then I've another class, that has a list of IMyCallbackas private field

public class MyClass
{
    private IList<IMyCallback> _Subscribers = new List<IMyCallback>();
    
    public void Sum(int a, int b)
    {
        int result = a + b;
        foreach (IMyCallback subscriber in _Subscribers)
        {
            subscriber.OnSum(a, b, result);
        }
    }
}

Pratically what it does is doing a sum between two numbers and notify all the service subscribers that it has performed the arithmetic operation a + b and the result is result.

Now I want to improve the performance and send each notification onto a Thread. What I usually do to avoid enclosure and memory problem with compiler is this:

public void Sum (int a, int b)
{
    int result = a + b;
    foreach (IMyCallback subscriber in _Subscribers)
    {
         new Thread((o) =>
         {
             object[] state = (object[])o;

             ((IMyCallback)state[0])
                   .OnSum((int)state[1], (int)state[2], (int)state[3]
         })
         {
             IsBackground = true
         }
         .Start(new object[] { subscriber, a, b, result });

    }
}

I want to focus the attention on the last part, when I pass all the parameters into an array of object and use it into the delegate. Am I doing correct? Is it a correct pattern or there are some issues I cannot understand?

Upvotes: 0

Views: 170

Answers (1)

Andy
Andy

Reputation: 13527

I added a comment to your question, but will try to reiterate what I was saying. Firstly: the question about passing an array of parameters as a generic object is perfectly fine. Because that method is the only method consuming that generic object array, it makes perfect sense.

The one thing that I am slightly concerned about is how you are spawning threads. If you have 10,000 subscribers, it's going to be rough on the ol' CPU. Since you are currently using synchronous code, I would use a Parallel.ForEach() on your subscribers.

public void Sum (int a, int b)
{
    var result = a + b;
    Parallel.ForEach (_Subscribers, subscriber =>
    {
        subscriber.OnSum(a,b,result);
    });
}

The advantage to this is you the the framework decide how many threads it needs to finish your job. If you have 5 subscribers or 10,000 -- it won't kill your CPU.

The beauty of this code as well is, if OnSum throws an exception, the caller will actually get it. With your current method, all exceptions will be lost forever.

ETA: Now, if you'd like to use async/await and Task, this is one way to safely do it:

public Task SumAsync (int a, int b)
{
    return Task.WhenAll(_Subscribers.Select(x=> Task.Run(() =>
    {
        x.OnSum(a, b, a + b);
    })));
}

This will also catch/throw all exceptions. I do not like this solution as it's not organically asynchronous. I like the use of Parallel.ForEach() for this solution.

Upvotes: 3

Related Questions