CODEWITHSUNDEEP
c#winformsasync-awaitbegininvoke
rhoonah
rhoonah

Reputation: 142

Better solution for GetAwaiter() and GetResult()

I have run into some legacy code Winforms code that basically does this...

private static bool _connected = false;
private static bool _closing = false;

static void Main(string[] args)
{
    // Stuff
    ThreadPool.QueueUserWorkItem(ThreadProc, null);
    // More stuff

    while (!_closing)
    {
        if (_connected)
        {
            break;
        }
        // Do stuff
        Application.DoEvents();
        Thread.Sleep(100);
        // Do stuff
    }

    if (_connected)
    {
        // Finally...
        Application.Run(new MyForm());
    }
}

private static void ThreadProc(object obj)
{
    while (!_closing)
    {
        // Stuff
        if (!Method2())
        {
            // Error
            _closing = true;
            return;
        }
        else
        {
            /// Success
            break;
        }
    }

    _connected = true;
}

private static async Task<SomeData> Method1()
{
    // There is a splash screen that gets displayed so we are
    // waiting for it to be available
    while (Application.OpenForms.Count == 0) Thread.Sleep(100);

    // So this gets run on the Windows GUI thread
    SomeData result = await (Application.OpenForms[0].Invoke(new Func<Task<SomeData>>(() => { SomethingAsync())));
    return result;
}

private static bool Method2()
{
    TaskAwaiter<SomeData> awaiter = Method1().GetAwaiter();
    while (!awaiter.IsCompleted)
    {
        // This appears to be why this code is done this way... 
        // to not block Windows events while waiting.
        Application.DoEvents();
        Thread.Sleep(100);
    }

    var SomeData = awaiter.GetResult();
    if (result == null) { return false; } // error

    // do something with result
    return true;
}

I have read up a bit on the negatives on using GetAwaiter().GetResult() and I'm searching for a better solution. I believe the reason this was done this way was so application events didn't block while Method1() executes since we are checking IsCompleted on the awaiter and once it is complete, we then get the result.

Is this a legit usage of GetAwaiter() and GetResult() or is there a better solution?

The above legacy code works and I am unsure as to whether this is a "good" solution to the problem or is a timebomb waiting to happen.

Clarification: The Method2 is called from a thread that was created using ThreadPool.QueueUserWorkItem() from inside the static Main() method, before the Application.Run() which is not called until after the thread terminates and is thus called on the same thread that entered the static Main() method. I have attempted to update the code with a simplified version of what I am dealing with that showcases the additional thread and when Application.Run() is invoked.

Purpose The purpose appears to be that when the application runs, it creates a thread that calls Method1 which basically does user login/authentication. Once authenticated, it attempts to connect to a server process using a JWT. The Main() thread is looping to wait for the connection thread to either succeed or error out while also calling Application.DoEvents(). I don't quite understand the purpose of the second thread.

Upvotes: 1

Views: 1294

Answers (1)

Charlieface
Charlieface

Reputation: 72279

No this is not the right way to go about this. You are running an infinite loop on a UI thread, putting it to sleep for 100ms, and then forcing it to wake and handle events then putting it to sleep again.

It is not a "time-bomb waiting to happen", but it is an inefficient method of waiting.

Instead, just use either async Task or async void which handles the waiting for you, but does not block the UI thread at all. It puts the continuation of the method (everything after the await) on a callback which only gets called once the await completes.

Note that async void should only be used in event handlers coming off the UI, otherwise use async Task and friends.

private static async Task Method2()
// alternatively
private static async void Method2()
{
    var data = await Method1();
    // do something with result
}

Upvotes: 2

Related Questions