1

Using C# 4.0, I would like to know a simple robust way to update the UI from within function foo() shown below. I need to update some textboxes that display the number of urls processed and the number of bad links found. The general approach I've taken on the async task is based on an approach I found here:

How to use HttpWebRequest (.NET) asynchronously?

Pseudo Code in my code-behind on the form:

1. populate a ConcurrentBag
2. Task.Factory.StartNew ( () =>
          Parallel.ForEach( ConcurrentBag, (d) =>
           {
             MyAsyncTask(d);

           }

And MyAsyncTask does:

          <snip>
          try
            {

             IAsyncResult result = myWebRequest.BeginGetResponse (
               new AsyncCallback( foo, state);



            ThreadPool.RegisterWaitForSingleObject(
            result.AsyncWaitHandle,
            new WaitOrTimerCallback(TimeoutCallback),
            state,
            (MSEC * 1000),  
            true
            );

           }
          catch (Exception ex) {}

            private void foo(IAsyncResult result)

            {
            RequestState state = (RequestState)result.AsyncState;
            WebRequest request = state.webRequest;
            HttpWebResponse response = (HttpWebResponse)request.EndGetResponse(result);

            //update the UI ??

            }
Community
  • 1
  • 1
Tim
  • 8,669
  • 31
  • 105
  • 183
  • Are you using Winforms, WPF? – ywm Jul 24 '13 at 17:09
  • This app is WinForms. – Tim Jul 24 '13 at 17:12
  • But if it is much easier to do in WPF it can be rewritten as WPF. – Tim Jul 24 '13 at 17:46
  • 2
    I hope that `catch() {}` isn't literally in your code. It only hides vital errors. – H H Jul 24 '13 at 20:25
  • You are mixing many technologies here. Some obsolete ones included. Rewrite this in a modern style and your troubles go away. – usr Jul 24 '13 at 21:08
  • @Henk Holterman: no, the catch is simplified here to keep the code brief. – Tim Jul 24 '13 at 23:50
  • @usr: hard to know what's modern and what's not since I've only used BackgroundWorker in the past, not having much need for threading. I'm using C# 4.0 and cannot take advantage of the newest features. – Tim Jul 24 '13 at 23:52

2 Answers2

1

First, a little code review:

  • You don't need a ConcurrentBag (a List<> will do)
  • You don't need the Parallel.ForEach(). BeginGetResponse() doesn't block
  • You probably don't even need the Factory.StartNew() Task, for the same reason
  • Using catch (Exception ex) {} only hides errors

Apply that and you have much simpler code.

And then, in foo(IAsyncResult result) you can update the UI in the normal way, using Control.Invoke(). This has been asked and answered many times like here.

Community
  • 1
  • 1
H H
  • 263,252
  • 30
  • 330
  • 514
  • I am trying to make as many web requests as possible in as short a time as possible. I thought that was what the Parallel.ForEach would accomplish. – Tim Jul 24 '13 at 23:53
  • Your main tool is BeginGetResponse(). Stacking other threading tools on top of that only complicates things. – H H Jul 25 '13 at 06:56
  • I would add: don't use the old APM pattern. Use task-based async IO (available on .NET 4). Remove RegisterWaitForSingleObject. – usr Jul 25 '13 at 10:47
  • OK, I will explore TAP. So far, I've read the following article: http://msdn.microsoft.com/en-us/library/hh873175.aspx – Tim Jul 25 '13 at 11:59
  • @Henk Holterman: So the TPL and TAP are not intended to work together? – Tim Jul 25 '13 at 12:11
  • @usr How to handle the timeout scenario in TAP pattern? – Tim Jul 25 '13 at 12:12
  • @Tim - TAP is part of the TPL, or at least adjacent to it. But whatever tech you use, don't stack needless layers of async on top of each other. Less is more. – H H Jul 25 '13 at 12:19
  • @Henk Holterman: Of course I agree with the principle. I'm out of my comfort zone at the moment since I do mostly T-SQL coding; but I've been tasked to write something that iterates 100K URLs and tests whether each URL is alive or dead, and have the process take as little time as possible. So I assumed multiple simultaneous asynchronous webrequests would be the way to go, and I also need to handle timeout scenarios where there's no response from the server within 'n' seconds. It has been recommended that I remove RegisterWaitForSingleObject. Lots of reading ahead of me. – Tim Jul 25 '13 at 12:31
0

You say:

I've been tasked to write something that iterates 100K URLs and tests whether each URL is alive or dead, and have the process take as little time as possible.

You don't need async for that. Just do this:

IEnumerable<string> urls = GetUrls(); //you provide this
var results =
 urls
 .AsParallel().WithDegreeOfParallelism(50)
 .Select(url => CheckUrl(url))
 .ToList();

PLINQ handles all parallelism and synchronization for you. Just provide a synchronous CheckUrl function. Store possible exceptions in whatever CheckUrl returns.

Async is best for WinForms/WPF or when you have tons of threads. 50 is not "tons".

usr
  • 168,620
  • 35
  • 240
  • 369
  • I did not realize that the checkUrl function could be synchronous, and supposed that all methods "downstream" had to be asynchronous as well. Users have asked for ongoing real-time tally of links processed and deadlinks encountered. This goes back to my original question -- where and how to update the UI textboxes to keep counters of URLs processed and deadlink-count? Is there any potential problem where more than one thread will be trying to update the UI control at the same time? – Tim Jul 25 '13 at 16:57
  • The requirement to keep realtime counters does not map so cleanly. I suggest you do that as part of the CheckUrl function. Use the `Invoke` pattern to call into the UI thread. As only one thread can call into it, the body of the callback is automatically single-threaded. – usr Jul 25 '13 at 20:49