Propagate a callback onto a separate thread

Question

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?

The only thing I don't like 100% is spawning off a thread for each Subscriber. What if you ended up with 1000 subscribers? Have you looked into TPL? For example: `Parallel.ForEach()`? BTW: The way you are passing objects to the thread is perfectly fine (my opinion). — Andy, Jul 31 '20 at 20:23
C# offers some higher level constructs to deal with these. An `event` for example is a multicast delegate which would allow you to avoid the loop to begin with, and a `Task` is [generally preferred](https://stackoverflow.com/a/13429164/2791540) over creating threads on your own. Do you have specific reasons for avoiding these? — John Wu, Jul 31 '20 at 21:27

Andy · Accepted Answer · 2020-07-31T21:16:16.437

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.

Propagate a callback onto a separate thread

1 Answers1