Unexpected block condition with Control.Invoke

Question

The following code is a simplified proof of concept. In a windows forms application there is a FormClosing event handler to perform some cleanup. The application launches some threads which write on the Form using Control.Invoke. I know I could use Control.BeginInvoke but I would like to understand better what happens and find another solution.

List<Thread> activeThreadlist;
volatile bool goOnPolling = true;
private void Form1_FormClosing(object sender, FormClosingEventArgs e)
    {
        goOnPolling = false;
        Thread.Sleep(1000);
        foreach (Thread element in activeThreadlist)
            if (element.IsAlive)
                element.Abort();

    }

A flag is set to false in order to stop the loop on the threads, they have time to terminate and if one is still alive it is terminated with abort.

Here is the code executed by the other threads:

while (goOnPolling)
{
    //some long elaboration here
    if (!goOnPolling) continue;
    aControl.Invoke(new Action(() =>
    {
        //display some result about the elaboration
    }));
    if (!goOnPolling) continue;
    //some long elaboration here    

}

In about 50% of the cases when the form is closed the threads blocks on Control.Invoke so they don't terminate during the sleep time and Abort is called. I thought checking the flag goOnPolling before calling Invoke would have been sufficient in 99.999% of the cases, but I was wrong. As stated in the code the threads do long elaborations (at least 100ms) so I expected goOnPolling was likely to change during them. Why am I wrong ? Is there another easy solution without recurring to BeginInvoke which creates an additional thread ?

Update

After reading the comments I realized the title was wrong. I initially wrote deadlock but it is only an unexpected (for me) block condition. In the beginning I couldn't understand why my threads were still alive after having waited their terminations for 1000ms, so I put an Abort to find out. I completely misunderstood the difference between Invoke and BeginInvoke, thanks for clarifying it.

@Jon B

I agree break is better suited than continue but that code is inside while (goOnPolling) block, so I could save only some cpu cycles, nothing more.

Any idea why so often goOnPolling changes during the early stages of Invoke ? If I perform a long elaboration it should happen there most of the time.

`BeginInvoke` shouldn't be creating additional threads, it just queues the work on the *existing* UI thread. Creating a new thread would be completely counter to what `BeginInvoke` is designed to do. — Bradley Uffner, Jul 06 '17 at 13:05
Are you sure about your last sentence? As far as I know both `Invoke` and `BeginInvoke` execute on the UI thread, only `Invoke` blocks before returning. i.e. `Invoke` = `BeginInvoke` + wait for execution. You may be confused with `delegate` `Invoke`/`BeginInvoke`, see https://stackoverflow.com/questions/229554/whats-the-difference-between-invoke-and-begininvoke . The only reason to use `Control.Invoke` is when you need the invoked action to complete before continuing on the calling thread. — Rotem, Jul 06 '17 at 13:05
I am 100% positive. `Invoke` queues the execution on the message loop, and then waits until it completes. `BeginInvoke` does the exact same thing, but doesn't block until completion. That's probably where your deadlock is coming from. Your UI thread is waiting on a child thread, and the child thread is waiting on the UI to complete. — Bradley Uffner, Jul 06 '17 at 13:10
@Filippo also, you should probably *never* call `Thread.Abort`. 99% (or even higher) of the time, when you see `Thread.Abort`, you are doing something very wrong. It can leave the application in such an unpredictable state that you can end up with all kinds of crazy bugs. See https://stackoverflow.com/questions/1559255/whats-wrong-with-using-thread-abort — Bradley Uffner, Jul 06 '17 at 13:17
On top of the other comments regarding `BeginInvoke` and `Abort`, I think instead of `if (!goOnPolling) continue;` you want `if (!goOnPolling) break;`. Or rather just stick with `while (goOnPolling)`. — Jon B, Jul 06 '17 at 13:43

score 0 · Answer 1 · answered Jul 06 '17 at 14:28

We know that these other threads want to get onto the UI thread through Invoke, and we know that you're tying up the UI thread in a Sleep.

This may be one of the few times where Application.DoEvents is the least-bad option here:

private void Form1_FormClosing(object sender, FormClosingEventArgs e)
{
    goOnPolling = false;
    for(int i=0;i<10;i++)
    {
        Application.DoEvents();
        Thread.Sleep(50);
    }
    //foreach (Thread element in activeThreadlist)
    //    if (element.IsAlive)
    //        element.Abort();
}

And also change the threads to cope with exceptions being thrown if they try to Invoke after the form has already been closed, rather than nastily trying to tear them down with an Abort.

As for why so many threads are in the state where they're wanting to Invoke, it's unclear from the code posted so far - but bear in mind that Invoke is serializing access to the UI thread. It's possible that all of the threads asked to Invoke well before the UI thread got to processing the windows message that translates to the "form closing" event, if access to the thread is heavily congested.

Unexpected block condition with Control.Invoke

1 Answers1