CODEWITHSUNDEEP
c#multithreading
Zion
Zion

Reputation: 1610

C# tasks and void methods

code

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using HtmlAgilityPack;
using System.Windows.Forms;

namespace WindowsFormsApplication1
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
            textBox1.Text = "place url hear";

        }

        private void Form1_Load(object sender, EventArgs e)
        {

        }

        private void textBox1_TextChanged(object sender, EventArgs e)
        {

        }

        private void button1_Click(object sender, EventArgs e)
        {

            Task.Factory.StartNew(() => get_url_contents(textBox1.Text)).ContinueWith(t => t.Id, TaskScheduler.FromCurrentSynchronizationContext());

        }


        private void get_url_contents(string url)
        {
            var doc = new HtmlWeb().Load(url);

            HtmlNodeCollection nodes = doc.DocumentNode.SelectNodes("//a");


            foreach(HtmlNode node in nodes)
            {
                listView1.Items.Add(node.InnerText);
            }

        }
        private void listView1_SelectedIndexChanged(object sender, EventArgs e)
        {

        }
    }
}

Im using windows forms and am practicing C#, Im fairly new to this language but know a little python.

basically, what im trying to do is you type a url on textBox1 and when you click button1 it will go to that url and extract all the link's text.

and append those results to listView1 however I keep getting this error

error message:

Additional information: Cross-thread operation not valid: Control 'listView1' accessed from a thread other than the thread it was created on.

how do we correct this?

Upvotes: 0

Views: 297

Answers (2)

Eric Lippert
Eric Lippert

Reputation: 660098

The (previously) accepted answer unfortunately continues down the wrong path you ended up on in the first place. The problem here is "I ended up needing to update the UI from the wrong thread by doing an asynchronous operation on a worker thread". The solution given is "have the worker thread marshal the call back to the UI thread". The better solution is don't end up trying to do UI work on a worker thread in the first place.

There is also no need to muck around with ContinueWith; in C# 5 and above we have asynchronous waiting.

Let's suppose that we genuinely do have work we want to perform on another thread. (This is suspicious; there is no reason why the high-latency operation here needs to go on another thread. It's not processor bound! But for the sake of argument, let's suppose we wish to download the HTML on another thread:

    async private void button1_Click(object sender, EventArgs e)
    {

Note that I have marked the method async. This does not make it run on another thread. This means "this method is going to return to its caller -- the message loop that dispatched the event -- before the work of the method is done. It will resume inside the method at some point in the future."

        var doc = await Task.Factory.StartNew(() => new HtmlWeb().Load(url));

What do we have here? We spawn off an asynchronous task that loads some HTML and returns a task. We then await that task. Awaiting the task means "return immediately to my caller -- again, the message loop that dispatched the button click -- to keep the UI running. When the task completes, this method will resume here, and obtain the value computed by the task."

Now the rest of the program is perfectly normal:

        HtmlNodeCollection nodes = doc.DocumentNode.SelectNodes("//a");
        foreach(HtmlNode node in nodes)
        {
            listView1.Items.Add(node.InnerText);
        }
    }

We're still on the UI thread; we're in a button click handler. The only work that was done on another thread was getting the HTML, and when it was available, we resumed right here.

Now, there are a few problems here.

What happens if the button is clicked again while we're waiting for the HTML to load? We try to load it again! It would be a good idea to turn the button off before the await and back on again after.

Also, as I alluded before, why are we spawning off a thread for a network-bound operation? You want to mail a letter to your aunt and get a reply; you don't have to hire a worker to take the letter to the mail box, mail it, and then sit by the mail box waiting for the reply. The vast bulk of the operation is going to be done by the post office; you don't need to hire a worker to do nothing but babysit the post office. Same thing here. The vast majority of the work will be done by the network; why are you hiring a thread to babysit it? Just find an asynchronous HTML loading method that gives you back a task, and await the task. HttpClient.GetAsync comes immediately to mind, though there may be others.

A third problem: we have created an object on the worker thread. Who says that it is safe to use it on the UI thread? There are many "threading models" that an object can have; in the COM world these are traditionally called "apartment" (you can only talk to me on the thread you created me on), "rental" (you can talk to me on any thread but you are required to ensure that no two threads try at the same time), "free" (anything goes, the object is safe) and a few others. The assumption here is that the object in question is safe for "rental" or better -- the read will not happen on the UI thread until the write is done on the worker thread. But if the object is really itself "apartment" then you have an object you can't talk to on any thread but the worker thread you just threw away. This is a potential real mess.

The moral of the story here is, first, keep everything on the same thread as much as you possibly can, and second don't turn your program inside out to make asynchrony work; just use await.

Upvotes: 8

srandppl
srandppl

Reputation: 571

You need to access it from the GUI thread. WInforms provide the invoke command for this ocassion.

listView1.Invoke(() => {
            foreach(HtmlNode node in nodes)
            {
                listView1.Items.Add(node.InnerText);
            }
}));

Upvotes: 3

Related Questions