Recursive Method Puts Strain on UI

Question

I have a method that utilises SendKeys.Send, waits for a few seconds using System.Threading.Thread.Sleep and then runs another method to check the colour of a pixel to see if it has changed. The method then runs again as it is called recursively.

This method needs to be able to run thousands of times before stopping. The Winform's UI seems to stop responding when this is running.

I tried to implement a background worker to take the strain off of the UI. I moved the recursive method's code over to the Do_Work event and called it with RunWorkerAsync but it crashed, reporting the following:

An exception of type 'System.InvalidOperationException' occurred in System.Windows.Forms.dll but was not handled in user code

Additional information: SendKeys cannot run inside this application because the application is not handling Windows messages.

What's the best way of moving the code away from the UI? I'm not very familiar with background workers so I may be doing it wrong.

You can use a background worker, just use `Invoke(new Action(SendKeys.Send), keys)` to dispatch the call on the UI thread. — Lou, Jun 13 '16 at 23:59
BGW isn't going to help you here. BGW exists to move long running, **CPU bound**, *non-UI* tasks to another thread. You have a long running *IO bound*, *UI* operation on your hands. — Servy, Jun 14 '16 at 00:02

piedar · Accepted Answer · 2016-07-28T20:38:07.287

1

Sounds like a case for async. Try replacing Thread.Sleep() with Task.Delay().

async void Button_Click(object sender, RoutedEventArgs e)
{
    await SendMyKeysAsync();
}

async Task SendMyKeysAsync()
{
    while (thePixelIsStillRed)
    {
        SendKeys.Send("whatever");
        await Task.Delay(TimeSpan.FromSeconds(1));
    }
}

This approach leaves the UI thread free to continue pumping messages during the delay period, without spawning any additional threads.

edited Jul 28 '16 at 20:38

answered Jun 14 '16 at 00:03

piedar

2,599
1
25
37

There isn't really any reason to make this method recursive. You're just notably increasing the resource consumption and using a non-idiomatic pattern for no real reason. – Servy Jun 14 '16 at 00:09
1

@Servy OP claims method calls itself recursively this way; method is just staying true to that. Not saying it's a good idea though – D. Ben Knoble Jun 14 '16 at 01:12
Turns out async was just what I needed. Thanks for that @piedar it has taken the strain off of the UI. Not sure why recursiveness would be a bad idea for this but it's getting the job done for me. – Chrayfish Jun 14 '16 at 13:01
@Chrayfish If recursion goes too deep, you'll eventually get a `StackOverflowException` from running out of stack space. For a simple method, this will probably only happen after many thousands of calls (and the compiler might help you out [under certain conditions](https://stackoverflow.com/a/29571052/1925996)), but it's usually best to avoid C# recursion if possible. – piedar Jun 14 '16 at 13:46
1

@Chrayfish A recursive `async` method has dramatically more resource consumption than a non-async method, as you're rebuilding the entire state machine of the method on every iteration. And again, it's very much non-idiomatic code in C#. It's not even an operation that conceptually makes sense as a recursive operation; it's not how most people are going to think of such a problem. – Servy Jun 15 '16 at 00:46
It will need to run potentially 1000s of times until the exiting condition is true. What would be the best route to take with this? – Chrayfish Jun 15 '16 at 08:44
Since we agree that recursion is not a good approach here, I removed the recursive example to avoid future confusion. – piedar Jul 28 '16 at 20:39

score 1 · Answer 2 · answered Jun 14 '16 at 00:05

Rather than a synchronous recursive method, you should write an asynchronous iterative method.

private async void Foo()
{
    while(ShouldKeepLooping())
    {
        SendKeys.Send(keyToSend);
        await Task.Delay(timespan.FromSeconds(2));
    }
}

Making the method recursive adds nothing; making it iterative removes stack pressure. By making the method asynchronous, rather than synchronous, you don't block the UI thread.

Recursive Method Puts Strain on UI

2 Answers2