Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Waiting for task to finish before closing form

How can I make the FormClosing event handler (which executes on UI thread) wait for a task, that does invokes on the very same form, to complete?

    private void Form1_FormClosing(object sender, FormClosingEventArgs e)
    {
        cancelUpdater.Cancel(); // CancellationTokenSource
        if (!updater.IsCompleted)
        {
            this.Hide();
            updater.Wait(); // deadlock if updater task is inside invoke
        }
    }
    private void Form1_Shown(object sender, EventArgs e)
    {
        cancelUpdater = new CancellationTokenSource();
        updater = new Task(() => { Updater(cancelUpdater.Token); });
        updater.Start();
    }
    void Updater(CancellationToken cancellationToken)
    {
        while(!cancellationToken.IsCancellationRequested)
        {
            this.Invoke(new Action(() => {
                ...
            }));
            //Thread.Sleep(1000);
        }
    }
like image 794
Chris Avatar asked Mar 17 '23 12:03

Chris


1 Answers

The right way to deal with this is to cancel the Close event, and then close the form for real when the task completes. That might look something like this:

private async void Form1_FormClosing(object sender, FormClosingEventArgs e)
{
    cancelUpdater.Cancel(); // CancellationTokenSource
    if (!updater.IsCompleted)
    {
        this.Hide();
        e.Cancel = true;
        await updater;
        this.Close();
    }
}

Now, in your comments, you wrote:

the Close() method called by a form B will return immediately and the changes that form B will make will crash the updater

Since you did not post any code related to "form B", it is not clear why or how it has anything to do with the current Form1 code. It's likely that there's a good way to fix this "form B" so that it cooperates better with the Form1 class and the object that's closing. But without seeing an actual good, minimal, complete code example that clearly shows this interaction, it's not possible to suggest how that would work.


Frankly, it is beyond being a really bad idea to block in any UI event handler. It is really important for the UI thread to be permitted to continue running unabated, and to do otherwise is to invite deadlock. You have of course here found one example of deadlock. But it is far from assured that even if you addressed this particular example, you could avoid all other instances of deadlock.

Blocking the UI thread is just asking for deadlock, among other problems.

That said, if you cannot resolve this issue with "form B" and really feel you must block the thread, you could make the cross-thread invoke use BeginInvoke() instead of Invoke() (which makes the invoke itself asynchronous, so that your "update thread" will be able to continue running and then terminate). Of course, should you do that, you will have to change the code to deal with the fact that by the time your invoked code is running, the form has been closed. That may or may not be a simple fix.


All that said, while I can't be sure lacking a good code example, I strongly suspect that you really should not have this updater task in the first place, and instead should be using the System.Windows.Forms.Timer class. That class is specifically designed for dealing with periodic events that have to be executed in the UI thread.

For example:

First, drag and drop a Timer object onto your form in the designer. By default, the name will be timer1. Then set the Interval property to the 1000 millisecond delay you're using in your task. Also, change your Updater() method so it's declared as timer1_Tick(object sender, EventArgs e) and use that as the event handler for the timer's Tick event.

Then, change your code so it looks something like this:

private void Form1_FormClosing(object sender, FormClosingEventArgs e)
{
    timer1.Stop();
}

private void Form1_Shown(object sender, EventArgs e)
{
    timer1.Start();
}

void timer1_Tick(object sender, EventArgs e)
{
    // All that will be left here is whatever you were executing in the
    // anonymous method you invoked. All that other stuff goes away.
    ...
}

Since the System.Windows.Forms.Timer class raises its Tick event on the UI thread, there are no thread race conditions. If you stop the timer in the FormClosing event, that's it. The timer's stopped. And of course, since the timer's Tick event is raised on the UI thread, there's no need to use Invoke() to execute your code.


IMHO, the above is the very best answer that can be offered given the information in the question. If you feel none of the above is useful or applies, please edit your question to provide all of the relevant details, including a good code example.

like image 163
Peter Duniho Avatar answered Mar 19 '23 19:03

Peter Duniho