6

Much to my discontent, I need to use a WebBrowser control in one of my apps.

One of the things I also need to do is wait for an element to become visible/class changes/etc, which happens well after the DocumentCompleted event is fired, making the event close to useless in my case.

So currently I have something like...

while (webBrowser.Document?.GetElementById("id")?.GetAttribute("classname") != "class")
{
    Application.DoEvents();
    Thread.Sleep(1);
}

Now I've read in multiple places that DoEvents() is evil and can cause lots of issues, so I thought about replacing it with Task.Delay() as such:

while (webBrowser.Document?.GetElementById("id")?.GetAttribute("classname") != "class")
{
    await Task.Delay(10);
}

So my question is, other than the obvious facts that the Thread.Sleep() will block events for 1ms and that the Task.Delay() has a bigger delay set in the example above, what are the actual differences between doing the two approaches, which is better and why?

PS: Please stick to the question, while I wouldn't necessarily mind other ideas on how to fix the WebBrowser control issue itself by using something else (js injection comes to mind), this is not the place to answer that, this question is about how these two bits of code differ and which would be considered better.

JohnUbuntu
  • 699
  • 6
  • 19
  • 1
    Here are plenty of reasons not to use DoEvents(): http://stackoverflow.com/a/11352575/80274 – Scott Chamberlain Feb 03 '16 at 21:35
  • Also, have you looked in to [this SO post](http://stackoverflow.com/questions/2777878/detect-webbrowser-complete-page-loading) perhaps `DocumentCompleted` is not firing early but instead is firing multiple times and you are just listening to the wrong one. – Scott Chamberlain Feb 03 '16 at 21:38
  • 1
    @ScottChamberlain I had read both links you pasted already. The one about `DocumentCompleted` doesn't quite apply to me (yes it does fire multiple times, unfortunately the URL is always the same every time it fires and even after the last time it fired the element is still not in the state I desire it to be). As for the dangers of `DoEvents()`, yep, had read it and understand, but my question is how the `Task.Delay()` differs from it. In other words, is it likely to suffer from the same caveats as the `DoEvents()`? How does it work any differently? – JohnUbuntu Feb 03 '16 at 21:42

3 Answers3

10

what are the actual differences between doing the two approaches, which is better and why?

The differences are in how messages are processed while waiting.

DoEvents will install a nested message loop; this means that your stack will have (at least) two message processing loops. This causes reentrancy issues, which IMO is the biggest reason to avoid DoEvents. There's endless questions about which kinds of events the nested message loop should process, because there's deadlocks on both sides of that decision, and there's no solution that's right for all applications. For an in-depth discussion of message pumping, see the classic Apartments and Pumping in the CLR blog post.

In contrast, await will return. So it doesn't use a nested message loop to process messages; it just allows the original message loop to process them. When the async method is ready to resume, it will send a special message to the message loop that resumes executing the async method.

So, await enables concurrency but without all the really difficult reentrancy concerns inherent in DoEvents. await is definitely the superior approach.

Basically there's an ongoing argument that DoEvents() is "better" because it doesn't consume any threads from the thread pool

Well, await doesn't consume any threads from the thread pool, either. Boom.

Community
  • 1
  • 1
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
3

While await Task.Delay(10) is executing the UI can process events. While Thread.Sleep(1); is running the UI is frozen. So the await version is better and will not cause any major issues.

Clearly, spinning in a busy loop has the risk of burning CPU and battery. You seem to be aware of those drawbacks.

usr
  • 168,620
  • 35
  • 240
  • 369
  • 1
    I am aware of the drawbacks and the differences you pointed out, but I'm more interested directly on the `DoEvents()` vs `Task.Delay()` rather than `Task.Delay()` vs `Thread.Sleep()` – JohnUbuntu Feb 03 '16 at 21:51
  • I suppose the other thing is also... bearing in mind that I'm constantly calling `await Task.Delay(10);` (until either it returns true or the timeout is reached) and that this will consume a thread (even if only for a brief moment) how will it affect the thread pool? – JohnUbuntu Feb 03 '16 at 21:53
  • Do you have any specific concerns? I have pointed out all differences between the two code snippets including the call to `DoEvents`. Not sure what to add since you seem to be aware of all the basic issues at play. `DoEvents` itself has no bad side-effects. It's just a sign of a bad architecture. – usr Feb 03 '16 at 21:53
  • While the delay runs no thread is being used for that. When the delay completes, a thread pool thread will be occupied for about a microsecond. That thread issues a BeginInvoke to the UI thread where your loop regains control. All of this consumes no resources to speak of. – usr Feb 03 '16 at 21:54
  • Basically there's an ongoing argument that DoEvents() is "better" because it doesn't consume any threads from the thread pool and that in this case since I'm waiting for a control that seems like the right thing to do. I personally am not of this opinion, but wanted to get the opinion of others. – JohnUbuntu Feb 03 '16 at 21:59
  • Now I understand your thinking. Sleep and DoEvents consume a thread while running but it's the UI thread which is always there. So if the entire app does not make any more use of the thread-pool (which is unlikely) then indeed you will have one more thread floating around (mostly idle). This is not an issue to speak of. Also, who knows what WinForms APIs activate the thread-pool themselves?! It's hard to avoid any use of the thread pool at all. – usr Feb 03 '16 at 22:04
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/102523/discussion-between-johnubuntu-and-usr). – JohnUbuntu Feb 03 '16 at 22:06
1

My 2 cents on this one:

Basically there's an ongoing argument that DoEvents() is "better" because it doesn't consume any threads from the thread pool and that in this case since I'm waiting for a control that seems like the right thing to do.

Broadly speaking, the WebBrowser control needs the UI thread to do some bits of its parsing/rendering job. By spinning a busy-waiting loop like yours with Application.DoEvents(); Thread.Sleep(1) on the UI thread, you essentially make this thread less available to other UI controls (including the WebBrowser itself). So, the polling operation from you example will most likely take longer to finish, than when you use an asynchronous loop with Task.Delay.

Moreover, the use of the thread pool by Task.Delay is not an issue here. A thread pool thread gets engaged here for a very short time, only to post an await completion message to the synchronization context of the main thread. That's because you don't use ConfigureAwait(false) here (correctly in this case, as you indeed need to do you polling logic on the UI thread).

If nevertheless you're still concerned about the use the thread pool, you can easily replicate Task.Delay with System.Windows.Forms.Timer class (which uses a low-priority WM_TIMER message) and TaskCompletionSource (to turn the Timer.Tick even into an awaitable task).

noseratio
  • 59,932
  • 34
  • 208
  • 486