Running an async task and invoking winform control

Question

If have this code:

Task[] tasks = new Task[3];


int index = 0;
foreach (KeyValuePair<string, int> packageId in PackageIds)
{
    await Task.Run(() => _logger.Log(packageId.Key));

    tasks[index] = Task.Factory.StartNew(() =>
    {
        Log(packageId.Key);
    });
    index++;
}

Task.WaitAll(tasks);
_logger.Log("all done");

This is the Log function:

private void Log(string message)
{
    ListViewItem lv = new ListViewItem
    {
        Text = $"{DateTime.Now:dd-MM-yyyy hh:mm:ss}"
    };
    lv.SubItems.Add(message);

    if (listView1.InvokeRequired)
        Invoke(new Action(() => Log(message)));
    else
        listView1.Items.Add(lv);
}

When I run it I can see the code tries to invoke the listView1. But after that, the application hangs and doesn't continue at all. When I remove the Log() and run it again, it works perfectly.

How can I run this code with the logging?

You obviously have infinite loop/recursion - `Log` function will call itself forever in case when `listView1.InvokeRequired = true` — Fabio, Jul 07 '17 at 04:37
_How can I run this code with the logging?_ - What you trying to achieve by logging message from another thread? Why not simply add logging message in listview inside `foreach` loop after await? — Fabio, Jul 07 '17 at 05:12
There is, as the code is right now, absolutely no reason to use a Task. Everything is still done on the UI thread. — Peter Bons, Jul 07 '17 at 06:53
Without a good [mcve], it's impossible to know how to best approach your scenario. That said, the code you posted is all kinds of wrong. The biggest issue (most likely responsible for the hang) is that you call `Task.WaitAll()`, while there is (apparently) code executing in other threads that will eventually want to call `Control.Invoke()` and use the thread you're blocking. Beyond that, it's not clear at all why you bother with the `Task` stuff anyway. There seems to be no benefit to it, and it's only making you confused. — Peter Duniho, Jul 07 '17 at 21:54

Harald Coppoolse · Accepted Answer · 2017-07-10T11:25:34.403

You are using the InvokeRequiered/Invoke pattern in a dangerous way.

The problem is that you use InvokeRequired to check if your thread can call functions from ListView1. If not, you don't use Invoke to call the ListView1 functions, but call your own function (this.Log).

The proper way to implement this pattern is to check if invoke is required is for your own form. Ask: this.InvokeRequired instead of listview1.InvokeRequired:

class MyForm : Form
{
    ...

    private void MyEventHandler(object sender, ...)
    {
        if (this.InvokeRequired)
        {
            Invoke(new MethodInvoker( () => this.MyEventHandler(sender, ...));
        }
    }
    else
    {
        ProcessMyEvent(...);
    }

If you use a debugger to step through this function, you'll see that invoke is required, and thus you invoke the same function again after which invoke is no longer required.

By the way, the reason to use MethodInvoker instead of Action is given in StackOverFlow: MethodInvoker vs Action for Control.BeginInvoke

Running an async task and invoking winform control

1 Answers1