Is there a better way to wait for queued threads?

Question

Is there a better way to wait for queued threads before execute another process?

Currently I'm doing:

this.workerLocker = new object(); // Global variable
this.RunningWorkers = arrayStrings.Length; // Global variable

// Initiate process

foreach (string someString in arrayStrings)
{
     ThreadPool.QueueUserWorkItem(this.DoSomething, someString);
     Thread.Sleep(100);
}

// Waiting execution for all queued threads
lock (this.workerLocker)  // Global variable (object)
{
     while (this.RunningWorkers > 0)
     {
          Monitor.Wait(this.workerLocker);
     }
}

// Do anything else    
Console.WriteLine("END");

// Method DoSomething() definition
public void DoSomething(object data)
{
    // Do a slow process...
    .
    .
    .

    lock (this.workerLocker)
    {
        this.RunningWorkers--;
        Monitor.Pulse(this.workerLocker);
    }
}

There are lots of ways. I think you need to read a good threading guide- albahari.com/threading. A producer consumer queue might be useful in your scenario, so might a Wait and pulse, or WaitHandles. – — RichardOD, Jun 25 '09 at 20:46
There is a race condition at this line: this.RunningWorkers--; Try using Interlocked.Decrement method for thread safe decrement. — SolutionYogi, Jul 01 '09 at 18:31
@SolutionYogi: RunningWorkers is protected by lock(this.workerLocker) — H H, Jul 02 '09 at 16:57

score 14 · Accepted Answer · edited Jul 11 '09 at 18:06

14

You likely want to take a look at AutoResetEvent and ManualResetEvent.

These are meant for exactly this situation (waiting for a ThreadPool thread to finish, prior to doing "something").

You'd do something like this:

static void Main(string[] args)
{
    List<ManualResetEvent> resetEvents = new List<ManualResetEvent>();
    foreach (var x in Enumerable.Range(1, WORKER_COUNT))
    {
        ManualResetEvent resetEvent = new ManualResetEvent();
        ThreadPool.QueueUserWorkItem(DoSomething, resetEvent);
        resetEvents.Add(resetEvent);
    }

    // wait for all ManualResetEvents
    WaitHandle.WaitAll(resetEvents.ToArray()); // You probably want to use an array instead of a List, a list was just easier for the example :-)
}

public static void DoSomething(object data)
{
    ManualResetEvent resetEvent = data as ManualResetEvent;

    // Do something

    resetEvent.Set();
}

Edit: Forgot to mention you can wait for a single thread, any thread and so forth as well. Also depending on your situation, AutoResetEvent can simplify things a bit, since it (as the name implies) can signal events automatically :-)

edited Jul 11 '09 at 18:06

Jader Dias

88,211
155
421
625

answered Jun 25 '09 at 20:50

Steffen

13,648
7
57
67

9

This won't work it you spin off more that 64 items at Windows only allows you to wait on up to 64 handles. – Sean Jun 29 '09 at 12:40
1

Thanks Sean, I didn't know this (and its important for my current project) – Steve Jun 29 '09 at 12:49
2

You forgot to append `resetEvent` into `resetEvents`. – Blindy Jun 29 '09 at 14:51
Blindy - not sure what you mean ? Sean - thanks, I didn't know that either, but yes it could be very important. Do you happen to know if it's only in 32 bit windows this is an issue ? – Steffen Jun 30 '09 at 05:31
Blindy means you didn't call resetEvents.Add(resetEvent) inside your loop – AgileJon Jun 30 '09 at 11:44
Aaah I see, yes that should obviously have been there. Guess I typed it up too fast. Thanks for notifying about it. – Steffen Jul 01 '09 at 05:39
1

I don't know why folks are not voting for Marc's 'Fork' class. It doesn't have the limitation of 64 handles like the above code and it's very clean. – SolutionYogi Jul 03 '09 at 17:35
I can't believe you guys are serious! Why on earth would you reinvent the wheel if Microsoft has done it for you?!! Check out my answer. – Andriy Volkov Jul 06 '09 at 01:46
zvolkov - I think most people would prefer not to download an extra DLL for this, as it's pretty basic stuff. Depending on a "3rd party" DLL for it, rather than using what's built into .Net, just seems "stupid" to me, as you have yet another piece of code that could be buggy (yes .Net can be buggy too, however it's less likely to happen, given the many users of it) No offense intended by the wording :-) – Steffen Jul 06 '09 at 05:02
It's not 3rd party, it's Microsoft, and it is included in .NET 4.0 – Andriy Volkov Jul 10 '09 at 13:50
@Blindy and @Steffen, added the missing line – Jader Dias Jul 11 '09 at 18:06
very heavy approach; + the limit of 64 – bohdan_trotsenko Jul 14 '09 at 06:38

score 13 · Answer 2 · edited May 23 '17 at 12:18

13

How about a Fork and Join that uses just Monitor ;-p

Forker p = new Forker();
foreach (var obj in collection)
{
    var tmp = obj;
    p.Fork(delegate { DoSomeWork(tmp); });
}
p.Join();

Full code shown on this earlier answer.

Or for a producer/consumer queue of capped size (thread-safe etc), here.

edited May 23 '17 at 12:18

Community

1
1

answered Jun 30 '09 at 11:42

Marc Gravell

1,026,079
266
2,566
2,900

I really love this particular idea. I am in process of incorporating it in my mini threading library. Thanks! – SolutionYogi Jul 03 '09 at 17:36

Andriy Volkov · Answer 3 · 2009-07-10T16:06:00.067

In addition to Barrier, pointed out by Henk Holterman (BTW his is a very bad usage of Barrier, see my comment to his answer), .NET 4.0 provides whole bunch of other options (to use them in .NET 3.5 you need to download an extra DLL from Microsoft). I blogged a post that lists them all, but my favorite is definitely Parallel.ForEach:

Parallel.ForEach<string>(arrayStrings, someString =>
{
    DoSomething(someString);
});

Behind the scenes, Parallel.ForEach queues to the new and improved thread pool and waits until all threads are done.

score 3 · Answer 4 · answered Jun 29 '09 at 15:36

I really like the Begin- End- Async Pattern when I have to wait for the tasks to finish.

I would advice you to wrap the BeginEnd in a worker class:

public class StringWorker
{
    private string m_someString;
    private IAsyncResult m_result;

    private Action DoSomethingDelegate;

    public StringWorker(string someString)
    {
        DoSomethingDelegate = DoSomething;
    }

    private void DoSomething()
    {
        throw new NotImplementedException();
    }

    public IAsyncResult BeginDoSomething()
    {
        if (m_result != null) { throw new InvalidOperationException(); }
        m_result = DoSomethingDelegate.BeginInvoke(null, null);
        return m_result;
    }

    public void EndDoSomething()
    {
        DoSomethingDelegate.EndInvoke(m_result);
    }
}

To do your starting and working use this code snippet:

List<StringWorker> workers = new List<StringWorker>();

foreach (var someString in arrayStrings)
{
    StringWorker worker = new StringWorker(someString);
    worker.BeginDoSomething();
    workers.Add(worker);
}

foreach (var worker in workers)
{
    worker.EndDoSomething();
}

Console.WriteLine("END");

And that's it.

Sidenote: If you want to get a result back from the BeginEnd then change the "Action" to Func and change the EndDoSomething to return a type.

public class StringWorker
{
    private string m_someString;
    private IAsyncResult m_result;

    private Func<string> DoSomethingDelegate;

    public StringWorker(string someString)
    {
        DoSomethingDelegate = DoSomething;
    }

    private string DoSomething()
    {
        throw new NotImplementedException();
    }

    public IAsyncResult BeginDoSomething()
    {
        if (m_result != null) { throw new InvalidOperationException(); }
        m_result = DoSomethingDelegate.BeginInvoke(null, null);
        return m_result;
    }

    public string EndDoSomething()
    {
        return DoSomethingDelegate.EndInvoke(m_result);
    }
}

score 3 · Answer 5 · answered Jul 02 '09 at 14:37

Yes, there is.

Suggested approach

1) a counter and a wait handle

int ActiveCount = 1; // 1 (!) is important
EventWaitHandle ewhAllDone = new EventWaitHandle(false, ResetMode.Manual);

2) adding loop

foreach (string someString in arrayStrings)
{
     Interlocked.Increment(ref ActiveCount);

     ThreadPool.QueueUserWorkItem(this.DoSomething, someString);
     // Thread.Sleep(100); // you really need this sleep ?
}

PostActionCheck();
ewhAllDone.Wait();

3) DoSomething should look like

{
    try
    {
        // some long executing code
    }
    finally
    {
        // ....
        PostActionCheck();
    }
}

4) where PostActionCheck is

void PostActionCheck()
{
    if (Interlocked.Decrement(ref ActiveCount) == 0)
        ewhAllDone.Set();
}

Idea

ActiveCount is initialized with 1, and then get incremented n times.

PostActionCheck is called n + 1 times. The last one will trigger the event.

The benefit of this solution is that is uses a single kernel object (which is an event), and 2 * n + 1 calls of lightweight API's. (Can there be less ?)

P.S.

I wrote the code here, I might have misspelled some class names.

The good thing here is that you only need 1 EWH, but Monitor is still preferable. The overhead of a EWH offsets the small savings of interlocked easily. — H H, Jul 02 '09 at 21:39

H H · Answer 6 · 2009-06-29T14:46:14.233

2

.NET 4.0 has a new class for that, Barrier.

Aside from that your method isn't that bad, and you could optimize a little by only Pulsing if RunningWorkers is 0 after the decrement. That would look like:

this.workerLocker = new object(); // Global variable
this.RunningWorkers = arrayStrings.Length; // Global variable

foreach (string someString in arrayStrings)
{
     ThreadPool.QueueUserWorkItem(this.DoSomething, someString);
     //Thread.Sleep(100);
}

// Waiting execution for all queued threads
Monitor.Wait(this.workerLocker);

// Method DoSomething() definition
public void DoSomething(object data)
{
    // Do a slow process...
    // ...

    lock (this.workerLocker)
    {
        this.RunningWorkers--;
        if (this.RunningWorkers == 0)
           Monitor.Pulse(this.workerLocker);
    }
}

You could use an EventWaithandle or AutoResetEvent but they are all wrapped Win32 API's. Since the Monitor class is pure managed code I would prefer a Monitor for this situation.

edited Jun 29 '09 at 14:46

answered Jun 25 '09 at 20:26

H H

263,252
30
330
514

I agree with Henk here. Your method is a relatively simple way to ensure that you only have as many working as you need, though I'd initialize your RunningWorkers variable explicitly, rather than through global means. The basic idea is sound, only you should pulse when ALL are done, not every time one is done. See http://www.albahari.com/threading/ for more ways of doing this in C#. You may like one of the ways there better. – Kevin Anderson Jun 25 '09 at 21:32
use Interlocked class instead of ++, --. – bohdan_trotsenko Jul 02 '09 at 14:49
modosansreves: not necessary, RunningWorkers is protected by the lock on workerLocker. – H H Jul 02 '09 at 15:27

score 0 · Answer 7 · answered Jun 25 '09 at 20:21

I'm not sure there is really, I've recently done something similar to scan each IP of a subnet for accepting a particular port.

A couple of things I can suggest that may improve performance:

Use the SetMaxThreads method of ThreadPool to tweak performance (i.e. balancing having lots of threads running at once, against the locking time on such a large number of threads).
Don't sleep when setting up all the work items, there is no real need (that I am immediately aware of. Do however sleep inside the DoSomething method, perhaps only for a millisecond, to allow other threads to jump in there if need be.

I'm sure you could implement a more customised method yourself, but I doubt it would be more efficient than using the ThreadPool.

P.S I'm not 100% clear on the reason for using monitor, since you are locking anyway? Note that the question is asked only because I haven't used the Monitor class previously, not because I'm actually doubting it's use.

score 0 · Answer 8 · answered Jul 03 '09 at 00:35

0

Use Spring Threading. It has Barrier implementations built in.

http://www.springsource.org/extensions/se-threading-net

answered Jul 03 '09 at 00:35

Kevin Stafford

160
1
1

Is there a better way to wait for queued threads?

8 Answers8

Suggested approach

Idea

P.S.

Linked