dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.41k stars 981 forks source link

Introduce InvokeAsync, ShowAsync, ShowDialogAsync #4631

Closed KlausLoeffelmann closed 2 months ago

KlausLoeffelmann commented 3 years ago

Rationale

[Scenario and API proposal updated to reflect the current eco system.]

A growing number of components in and for WinForms requires to asynchronously marshal an async method to run on the UI-Thread. These are for example APIs around WebView2, projected native Windows 10 and 11 APIs or async APIs from modern libraries for example for Semantic Kernel.

Other scenarios make it necessary to show a Form, a Popup or a MessageBox asynchronously, to be in alignment of other UI stacks like WinUI or .NET MAUI and share/adapt their ViewModel implementations for migration and modernization purposes.

One example where we need to use these APIs is when we need to implement a Login-Dialog in WinForms to authenticate a user on a WebAPI which represents the WinForms App's backend, and which uses Windows WebView2 control and MSALs ICustomWebUI interface for acquiring an authorization code asynchronously.

Image

Since a modern architecture would it make necessary to call this Dialog via an UI-Service from a ViewModel, we would need to show this Form either with ShowAsync or ShowDialogAsync. Inside of the Form, when we would need to navigate to a specific URL and to honor the implementation of ICustomWebUI, we would need to call NavigateAsyncTo which would need to run a) asynchronously and b) on WinForms's UI Thread:

public partial class FormWebLogin : Form, ICustomWebUi
{
    public FormWebLogin()
    {
        InitializeComponent();
        InitializeBrowser();
    }

    /// <summary>
    ///  Implements the <see cref="ICustomWebUi.AcquireAuthorizationCodeAsync(Uri, Uri, CancellationToken)"/> method.
    /// </summary>
    public async Task<Uri> AcquireAuthorizationCodeAsync(
        Uri authorizationUri,
        Uri redirectUri,
        CancellationToken cancellationToken)
    {
        _redirectUri = redirectUri;

        // NavigateToAsync must be called asynchronously AND need to run on the UI thread.
        return await InvokeAsync<Uri>(
            () => NavigateToAsync(authorizationUri),
            cancellationToken);
    }

The following is a demo which utilizes InvokeAsync with the Periodic Timer, demos parallelized rendering into the WinForms's GDI+ Graphics surface and also shows off the new Windows title bar customizing (which is part of the Dark Mode features):

Image

API Suggestion:

Note: We suggest for .NET 9 to implement the new APIs under the Experimental attribute, so we can collect and react to respective feedback, and then introduce the new APIs finally in .NET 10.

namespace System.Windows.Forms;

public static partial class TaskDialog
{
    public static Task<TaskDialogButton> ShowDialogAsync(nint hwndOwner, TaskDialogPage page, TaskDialogStartupLocation startupLocation, CancellationToken cancellationToken);
    public static Task<TaskDialogButton> ShowDialogAsync(IWin32Window win32Window, TaskDialogPage page, TaskDialogStartupLocation startupLocation, CancellationToken cancellationToken);
    public static Task<TaskDialogButton> ShowDialogAsync(TaskDialogPage page);
    public static Task<TaskDialogButton> ShowDialogAsync(TaskDialogPage page, CancellationToken cancellationToken);
    public static Task<TaskDialogButton> ShowDialogAsync(TaskDialogPage page, TaskDialogStartupLocation startupLocation);
    public static Task<TaskDialogButton> ShowDialogAsync(TaskDialogPage page, TaskDialogStartupLocation startupLocation, CancellationToken cancellationToken);
}

public partial class Form
{
    public Task ShowAsync(IWin32Window? owner = null, CancellationToken cancellationToken = default);
    public Task<DialogResult> ShowDialogAsync();
    public Task<DialogResult> ShowDialogAsync(IWin32Window owner);
    public Task<DialogResult> ShowDialogAsync(CancellationToken cancellationToken);
    public Task<DialogResult> ShowDialogAsync(IWin32Window owner, CancellationToken cancellationToken);
}

public partial class Control
{
    // Awaiting the execution of the UI-Thread-marshalled `Action` or `Func`. 
    // If `Func` returns `Task[<TResult>]` it is also awaited.
    public Task InvokeAsync(Action action);
    public Task InvokeAsync(Action action, CancellationToken cancellationToken);
    public Task InvokeAsync(Func<Task> asyncFunc, CancellationToken cancellationToken);
    public Task<T> InvokeAsync<T>(Func<Task<T>> asyncFunc, CancellationToken cancellationToken);
    public Task<TResult> InvokeAsync<TResult>(Func<TResult> syncFunction);
    public Task<TResult> InvokeAsync<TResult>(Func<TResult> syncFunction, CancellationToken cancellationToken);
}
KlausLoeffelmann commented 3 years ago

[Update: This implementation is no longer valid.]

Oh, and this is the implementation I did so far with which I used in the demo:

        private async Task<T> InvokeAsync<T>(
            Delegate invokeDelegate,
            TimeSpan timeOutSpan = default,
            CancellationToken cancellationToken = default,
            params object[] args)
        {
            var tokenRegistration = default(CancellationTokenRegistration);
            RegisteredWaitHandle? registeredWaitHandle = null;

            try
            {
                TaskCompletionSource<bool> taskCompletionSource = new();
                IAsyncResult? asyncResult = BeginInvoke(invokeDelegate, args);

                registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject(
                    asyncResult.AsyncWaitHandle,
                    new WaitOrTimerCallback(InvokeAsyncCallBack),
                    taskCompletionSource,
                    timeOutSpan.Milliseconds,
                    true);

                tokenRegistration = cancellationToken.Register(
                    CancellationTokenRegistrationCallBack,
                    taskCompletionSource);

                await taskCompletionSource.Task;

                object? returnObject = EndInvoke(asyncResult);
                return (T)returnObject;
            }
            finally
            {
                registeredWaitHandle?.Unregister(null);
                tokenRegistration.Dispose();
            }

            static void CancellationTokenRegistrationCallBack(object? state)
            {
                if (state is TaskCompletionSource<bool> source)
                    source.TrySetCanceled();
            }

            static void InvokeAsyncCallBack(object? state, bool timeOut)
            {
                if (state is TaskCompletionSource<bool> source)
                    source.TrySetResult(timeOut);
            }
        }
    }
weltkante commented 3 years ago
public Task InvokeAsync(
    Func<Task> invokeDelegate,
    TimeSpan timeOutSpan = default,
    CancellationToken cancellationToken = default,
    params object[] args) 

The args make no sense, Func<Task> takes no arguments and if you pass anything but an empty array you'll get an exception (same for the other overload).

Also do you really want BeginInvoke behavior which always interrupts control flow (posting to the message loop) even if you are already on the UI thread? I mean its ok to do if you document it, people can do their own extension method which checks InvokeRequired just like they are doing currently. I only want to make sure its discussed which variant is expected to be the more common use case, immediate execution when already being on the UI thread, or always post like BeginInvoke.

If you divert from BeginInvoke behavior and execute the callback inline on the UI thread then you should also discuss returning ValueTask instead of class Task. I've not got any experience with ValueTask patterns so I'm just throwing this into the discussion for consideration not because its necessarily a good idea. (If you have BeginInvoke behavior and always post there is no point of ValueTask though).


Also (without seeing how the private InvokeAsync is called) I suspect this is an overly complex and probably bad approach to implement InvokeAsync, but shouldn't derail the API discussion, implementation can be discussed separately if desired. (For example I'm missing unrolling and awaiting of the Task returned by Func<Task> but maybe its done in the caller. Without having this the Task returned by InvokeAsync would complete before the async callback runs to completion.)

Cancellation also seems suspect

Thats not to say cancellation isn't useful to have in this API, if you do BeginInvoke behavior of posting you may want to cancel your posted callback before it runs.

terrajobst commented 3 years ago
namespace System.Windows.Forms
{
    public partial class Control
    {
        public Task InvokeAsync(
            Func<Task> invokeDelegate,
            TimeSpan timeOutSpan = default,
            CancellationToken cancellationToken = default,
            params object[] args
        );
        public async Task<T> InvokeAsync<T>(
            Func<Task<T>> invokeDelegate,
            TimeSpan timeOutSpan = default,
            CancellationToken cancellationToken = default,
            params object[] args
        );
    }
}
stephentoub commented 3 years ago

Just skimming the post now...

Am I understanding correctly that the timeout and CancellationToken don't actually impact the execution of the invokeDelegate, rather they cause the returned task to transition to a completed state even when the invokeDelegate may still be executing?

If so, that's exactly what the new WaitAsync methods will do: https://github.com/dotnet/runtime/pull/48842 e.g. if there's an existing Task-returning InvokeAsync method, you wouldn't need these overloads, and could instead just do:

await InvokeAsync(() => ...).WaitAsync(cancellationToken, timeout);

That WaitAsync takes the task from the InvokeAsync and returns a task that also incorporates cancellation / timeout, e.g. if cancellation is requested, the task returned from WaitAsync will be canceled even though the task returned from InvokeAsync may still be running.

noseratio commented 3 years ago

Oh, and this is the implementation I did so far with which I used in the demo:

Alternatively, I've been using the following version, modified to support timeOutSpan. I my experience, I never needed timeOutSpan, as CancellationToken was always good enough. Removing timeOutSpan could make it a bit shorter and perhaps a bit more performant, as the linked CancellationTokenSource would be gone too.

public static async Task<T?> InvokeAsync<T>(
    this Control @this,
    Delegate invokeDelegate,
    TimeSpan timeOutSpan = default,
    CancellationToken  = default,
    params object?[]? args)
{
    using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
    if (timeOutSpan != default)
    {
        cts.CancelAfter(timeOutSpan);
    }

    var tcs = new TaskCompletionSource<T?>(TaskCreationOptions.RunContinuationsAsynchronously);
    @this.BeginInvoke(new Action(() =>
    {
        try
        {
            // don't invoke the delegate if cancelled
            cts.Token.ThrowIfCancellationRequested();
            tcs.TrySetResult((T?)invokeDelegate.DynamicInvoke(args));
        }
        catch(Exception ex)
        {
            tcs.TrySetException(ex);
        }
    }), null);

    using (cts.Token.Register(() => tcs.TrySetCanceled()))
    {
        return await tcs.Task;
    }
}

Edited: addressing the comments, here is an updated version that supports both regular and async delegates for invokeDelegate.

weltkante commented 3 years ago

Alternatively, I've been using the following version

Contrary to the original method (where I couldn't see the caller) this is public, thus definitely broken, because it doesn't unwrap and await the task returned by Func<Task> or Func<Task<T>> but instead returns a Task<Task>

When passing an actual async function the Task returned by InvokeAsync completes before the invokeDelegate completes.

In other words, your InvokeAsync does not support async callbacks, it supports calling synchronous callbacks in an async way. While in isolation it may be valid its different from how the rest of the dotnet framework developed.

I believe you shouldn't be writing a catch-all method handling arbitrary delegates. While you could try inspecting the return type of the delegate and "guess" based on its type whether you have to await it, that only works for Task and falls short for other cases. ValueTask and custom implementations will again cause the same problem of completing before the callback completes.

The IMHO correct solution is to only accept Func<Task> and Func<Task<T>>, by forcing the delegate signature to return Task (instead of accepting arbitrary delegates) you ensure the user gets a compiler error instead of runtime bugs. He has then a simple way to fix this compile error, by marking his method async and doing an await if necessary. This will always convert his method into the proper Task-returning signature.

noseratio commented 3 years ago

Contrary to the original method (where I couldn't see the caller) this is public, thus definitely broken, because it doesn't unwrap and await the task returned by Func<Task> or Func<Task<T>> but instead returns a Task<Task>

I guess I see what you mean. That version was for invoking synchronous delegates, and the real code accepts an Action (edited: Func<T?>), not an untyped Delegate:

static async Task<T?> InvokeAsync<T>(this Control @this, Func<T?> func, CancellationToken cancellationToken)

As for async lambdas, I use a different override (similar to how Task.Run does it):

public static async Task<T?> InvokeAsync<T>(
    this Control @this,
    Func<CancellationToken, Task<T?>> asyncFunc,
    CancellationToken cancellationToken = default)
{
    var tcs = new TaskCompletionSource<T?>(TaskCreationOptions.RunContinuationsAsynchronously);
    @this.BeginInvoke(new Action(async () =>
    {
        // we use async void on purpose here
        try
        {
            cancellationToken.ThrowIfCancellationRequested();
            tcs.TrySetResult((T?)await asyncFunc(cancellationToken));
        }
        catch (Exception ex)
        {
            tcs.TrySetException(ex);
        }
    }), null);

    using (cancellationToken.Register(() => tcs.TrySetCanceled()))
    {
        return await tcs.Task;
    }
}

The IMHO correct solution is to only accept Func and Func<Task>, by forcing the delegate signature to return Task (instead of accepting arbitrary delegates) you ensure the user gets a compiler error instead of runtime bugs. He has then a simple way to fix this compile error, by marking his method async and doing an await if necessary. This will always convert his method into the proper Task-returning signature.

Totally agree and that's how I do it in my projects, following the pattern of many others existing APIs in .NET.

Edited, as a matter of fact, @KlausLoeffelmann's version throws for the following code due to the same reason:

private async void Form1_Load(object sender, EventArgs e)
{
    // we're on UI thread
    await Task.Run(async () =>
    {
        // we're on thread pool
        var res = await this.InvokeAsync<bool>(new Func<Task<bool>>(async () => 
        {
            // we're on UI thread
            await Task.Delay(5000);
            return true;
        }));

        // we're on thread pool again
        await Task.Delay(1000);
    });

    // back on UI thread
    MessageBox.Show("Done");
}

BeginInvoke is unaware of the fact that the delegate is async and returns to the caller before Task.Delay(5000) is completed. This of course is solved with a proper override like I mentioned in this post.

noseratio commented 3 years ago

Contrary to the original method (where I couldn't see the caller) this is public, thus definitely broken, because it doesn't unwrap and await the task returned by Func<Task> or Func<Task<T>> but instead returns a Task<Task>

When passing an actual async function the Task returned by InvokeAsync completes before the invokeDelegate completes.

In other words, your InvokeAsync does not support async callbacks, it supports calling synchronous callbacks in an async way. While in isolation it may be valid its different from how the rest of the dotnet framework developed.

The original method seems to be broken too for async lambdas, as I've shown above. However, it's still possible to make it work for async lambdas disguised as untyped Delegates. It's a questionable design, but it's also the legacy of BeginInvoke.

Here's a take at that, it works for both regular and async delegates:

public static async Task<T?> InvokeAsync<T>(
    this Control @this,
    Delegate invokeDelegate,
    TimeSpan timeOutSpan = default,
    CancellationToken cancellationToken = default,
    params object?[]? args)
{
    using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
    if (timeOutSpan != default)
    {
        cts.CancelAfter(timeOutSpan);
    }

    var tcs = new TaskCompletionSource<T?>(TaskCreationOptions.RunContinuationsAsynchronously);

    async void Invoker() 
    {
        // async void makes sense here
        try
        {
            cts.Token.ThrowIfCancellationRequested();

            var result = invokeDelegate.DynamicInvoke(args);
            // if the result is a Task, await it
            if (result is Task task)
            {
                await task;
                tcs.TrySetResult(((Task<T?>)task).GetAwaiter().GetResult());
            }
            else
            {
                tcs.TrySetResult((T?)result);
            }
        }
        catch (Exception ex)
        {
            tcs.TrySetException(ex);
        }
    }

    @this.BeginInvoke(new Action(Invoker));

    using (cts.Token.Register(() => tcs.TrySetCanceled()))
    {
        return await tcs.Task;
    }
}
KlausLoeffelmann commented 3 years ago

@weltkante

Also do you really want BeginInvoke behavior which always interrupts control flow (posting to the message loop) even if you are already on the UI thread?

Absolutely. One of the features I use Invoke for, and which I find super important in scenarios, where I want dedicatedly a method call to be getting event characteristics through scheduling it on the message queue. Even if not fired off from a different thread. I’ll add a rational for that as well in the nearer future, and I am really interested in a discussion about alternative approaches for those scenarios.

weltkante commented 3 years ago

I am really interested in a discussion about alternative approaches for those scenarios.

await Task.Yield() will achieve the same thing and I personally prefer having it separate from InvokeAsync, but I'm not against keeping it in line with BeginInvoke and having users check InvokeRequired manually. Either way is fine, you always can implement the other semantic as extension method if you prefer it. I just want some discussion since from my perspective it feels like I want the inlined case more often, so getting other peoples perspective may be good.

// simplified pseudocode for how you would implement one if you have the other

// if InvokeAsync behaves as BeginInvoke as proposed you can check InvokeRequired
public static async Task InvokeAsync_Inline(Func<Task> callback)
{
  if (InvokeRequired)
    await InvokeAsync_Yield(callback);
  else
    await callback();
}

// the currently proposed semantic could be reimplemented as extension method if InvokeAsync would inline
public static async Task InvokeAsync_Yield(Func<Task> callback)
{
  if (!InvokeRequired)
  {
    await Task.Yield();
    await callback(); // since InvokeAsync_Inline(callback) would inline it anyways
  }
  else
  {
    await InvokeAsync_Inline(callback);
  }
}

If InvokeAsync does inline when already on the UI thread I would not expect people to write above extension method though, I'd expect people to explicitly call await Task.Yield(); if they want to interrupt the flow of execution. Its a bit like a less evil Application.DoEvents() (less evil because it forces the method to be async, making it clear to the caller that there is reentrancy)

RussKie commented 3 years ago

I wonder if something like @kevingosse proposed in his article would allow us achieve the desired in a different way?

weltkante commented 3 years ago

The linked article is more like vs-threading using the pattern await SwitchToMainThread() and await SwitchToThreadPool() (names changed for clarity)

This pattern means you explicitly "change" threads by awaiting a custom awaiter, which will either be a no-op if you are on the right thread, or suspend execution and resume it on the right thread (by posting to the SynchronizationContext, which is equivalent to a BeginInvoke).

Having explicit async-await thread switches can be nice for readability in some scenarios, but make it worse in others since there is a hidden implication of the "current thread type" you have to keep in your mind when reading the source. We've been using the vs-threading library ever since it was open sourced (works for both WinForms and WPF and also solves deadlock issues you'd have with naive implementation of such thread switches), but I believe this approach is orthogonal to a one-off InvokeAsync and both approaches have reasons to exist.

noseratio commented 3 years ago

I wonder if something like @kevingosse proposed in his article would allow us achieve the desired in a different way?

That's a very interesting approach, thanks for the link. I personally like @kevingosse's initial idea, without the added AsyncMethodBuilder machinery:

private async Task UpdateControls()
{
    await Dispatcher.SwitchToUi();

    TextTime.Text = DateTime.Now.ToLongTimeString();
}

This kind of tells the intention right away, in a very readable way, IMHO. A similar API has been already proposed for .NET 6.0.

A potential problem I see with this is that we need to somehow remember the UI thread's task scheduler (TaskScheduler.FromCurrentSynchronizationContext). If we are already on a non-UI thread, and all we have is a Control object, the best thing we could then is to still use Control.Invoke to get it.

That said, a well-designed UI app should be updating the UI indirectly via a proper pattern (MVP/MVVM/MVU). So, we'd have a view model object, which could keep a references to the UI thread's task scheduler. That's where something like await uiTaskScheduler.SwitchTo() might be very useful.

weltkante commented 3 years ago

A potential problem I see with this is that we need to somehow remember the UI thread's task scheduler

Only a problem if you want to build it as an external library, WinForms internally can have all the magic it wants, since it is providing the SynchronizationContext in the first place. That said BeginInvoke is a very cheap way to switch threads as long as you don't use the IAsyncResult (which is one reason why the implementation in the OP is bad). - In fact the SynchronizationContext and TaskScheduler use BeginInvoke for their implementation, so its the lowest primitive you can currently get.

However note that await control.SwitchToThreadAsync() (or whatever you want to name it) will have massively different semantics and programming styles than await control.InvokeAsync(... something to do ...):

await control.SwitchToThreadAsync()

control.InvokeAsync(async () => { ... })

Like said above, I believe both approaches are valid, have their own advantages and can live alongside each other.

RussKie commented 3 years ago
  • you can already have something very close to this today (just not a method on Control) by referencing Microsoft.VisualStudio.Threading nuget package. don't mind the "Visual Studio" in the name, its just that this is where it came from, its available to everyone now and is open source

Git Extensions has been using Microsoft.VisualStudio.Threading as well (https://github.com/gitextensions/gitextensions/pull/4601, thanks to @sharwell), but it required additional tweaks and plumbing (e.g. ControlThreadingExtensions and ThreadHelper), and then further more tweaks to work reliably in UI tests. Those aren't the most straight forward implementations, and likely a lot of (Windows Forms) developers will find those intimidating.

sharwell commented 3 years ago

My main concern with the InvokeAsync proposal is the same concern I still have with the Git Extensions approach: it's not obvious how these methods should tie in with closing/disposing forms, and I'm not convinced they will make it any easier for developers to address those concerns.

I will say: once the plumbing for vs-threading is in place, the code using that approach is dramatically cleaner than code trying to use InvokeAsync-style approaches.

noseratio commented 3 years ago

it's not obvious how these methods should tie in with closing/disposing forms, and I'm not convinced they will make it any easier for developers to address those concerns.

I think, the same concern applies to any async method which does anything on the UI thread.

I usually tackle this with the "main" cancellation token source which is triggered when the form is closing/disposing. With InvokeAsync, I imagine it could look like this:


async Task WorkAsync(CancellationToken outerToken, SomeForm form) {
    using var cts = CancellationTokenSource.CreateLinkedTokenSource(outerToken, form.MainToken);
    // ...
    await form.InvokeAsync(UpdateUiAsync, cts.Token);
    // ...
}

async Task UpdateUiAsync(CancellationToken token) {
    // e.g., we may need a delay for some debounce/throttle logic
    await Task.Delay(100, token); // this should throw if the form is gone 
    UpdateSomeUI();
    token.ThrowIfCancellationRequested(); // this should throw if the form is gone now
    UpdateSomeOtherUI();
}

InvokeAsync should be passing a cancellation token to its callback, whether the callback is sync or async. That's what I proposed in this code snippet.

I believe it's a good practice to make cancellation as "all way down" propagating as async/await itself should be.

KlausLoeffelmann commented 3 years ago

So, after a short email-exchange with @Pilchie, I tried the following based on a question he asked and I am wondering: what about this approach?

        public async Task InvokeAsync(
            Func<Task> invokeDelegate)
        {
            var asyncResult = BeginInvoke(invokeDelegate, args);
            _ = await Task<object>.Factory.FromAsync(asyncResult, EndInvoke);
        }

And, apart from the fact that discoverability of this as an alternative is obviously a problem, I am wondering: is it an alternative? What problems do you see here? If using this is OK, would we still need InvokeAsync?

noseratio commented 3 years ago

So, after a short email-exchange with @Pilchie, I tried the following based on a question he asked and I am wondering: what about this approach?

@KlausLoeffelmann I don't think this is going to work well, for the same reason I brought up here.

BeginInvoke is unware of async callbacks. As far as BeginInvoke is concerned, the call is done when a Func<Task> delegate returns a Task, not when the Task it returns is completed. To illustrate the problem, using the version of InvokeAsync you proposed above:

await Task.Run(async () =>
{
    // we're on thread pool
    await control.InvokeAsync(new Func<Task<bool>>(async () => {
        // we're on UI thread
        await Task.Delay(1000);
        Console.WriteLine("Updating");
        this.Text = "Updated";
        throw new Exception("Oh no");
    }));
});
Console.WriteLine("Done");

The output will be:

Updating
Done
Updated
<an exception is thrown but goes unobserved and lost>

I'd expect:

Updating
Updated
<an exception is thrown and propagated outside>

One other point. If I'm not mistaken, the Factory.FromAsync(asyncResult, EndInvoke) approach still makes use of IAsyncResult.AsyncWaitHandle, which leads to the creation of a ManualResentEvent event and all the overhead of asynchronously waiting for it to be signaled.

I keep pitching the TaskCompletionSource-based approach I proposed in various forms in this thread. It doesn't relay upon AsyncWaitHandle.

weltkante commented 3 years ago

Just to be clear, for semantics I expect control.InvokeAsync(callback) to behave exactly as Task.Run(callback) - just on the controls UI thread instead of the thread pool. That means the returned Task is used for observing the callbacks completion and result, including exceptions. Inventing new semantics just used by WinForms will be confusing to users.

Furthermore, don't be lazy and try to offload the implementation of InvokeAsync onto already existing primitives, WinForms doesn't have code that supports async/await (or even understands Task) so there is nothing you can offload onto. You can and should use BeginInvoke to switch threads (because as explained above its the lowest level primitive WinForms has) - but do not use the IAsyncResult in any form, allocating that ManualResetEvent is extremely expensive (it needs interop and has a finalizer) and will be noticeable since its done for every call. The IAsyncResult is a leftover for compatibility and should not be used in modern code.

Shidell commented 3 years ago

Also do you really want BeginInvoke behavior which always interrupts control flow (posting to the message loop) even if you are already on the UI thread?

@weltkante Doesn't BeginInvoke get translated into an (asynchronous) Win32 PostMessage? (Why would that interrupt control flow, even if you are already on the UI thread?) Are you referring to the lack of an InvokeRequired check, and simply calling BeginInvoke with a delegate every time, regardless of whether or not we're already on the UI thread?

weltkante commented 3 years ago

Doesn't BeginInvoke get translated into an (asynchronous) Win32 PostMessage?

yes it does

Why would that interrupt control flow, even if you are already on the UI thread?

because if you are on the UI thread, you will stop running your code and only resume once all other pending messages are processed

Are you referring to the lack of an InvokeRequired check, and simply calling BeginInvoke with a delegate every time, regardless of whether or not we're already on the UI thread?

I'm talking about the planned semantic of InvokeAsync. There's a design decision of whether InvokeAsync will immediately execute the callback when already on the UI thread (no BeginInvoke call, but invoke the callback immediately), or whether to always call BeginInvoke even if already on the UI thread. The latter will result in pausing the flow of execution in the async caller to await until the message queue is drained. (Even if its already drained it will bubble back to the message loop and may execute other code in the caller before the posted message is resuming execution.)

Like explained above you can create either version from the other and I wanted to start a discussion which one is the more commonly desired behavior and thus the desired "out of the box" experience.

As far as precedence goes:

My personal opinion is that executing the callback immediately is more natural for async/await code because

noseratio commented 3 years ago

I wanted to start a discussion which one is the more commonly desired behavior and thus the desired "out of the box" experience.

In this light, I'd make InvokeAsync always async, regardless of whether it's already on the UI thread or not. Knowing it's always async is less cognitive load, especially given it's in the name of the method.

I think I've mentioned a few times already how JavaScript Promises always resolve asynchronously (even for something simple as await true), and that doesn't seem to be a problem in that ecosystem.

weltkante commented 3 years ago

Knowing it's always async is less cognitive load, especially given it's in the name of the method.

Yes this is a significant argument that has to be weighted in, though a counterargument is that most other awaits you do will inline if its await-condition is aready fulfilled, so InvokeAsync would stand out and not fully integrate into the async/await world in order to be more in line with historic behavior. (I'm explicitly not judging here which is "better", leaving this to others, just presenting arguments).

noseratio commented 3 years ago

though a counterargument is that most other awaits you do will inline if its await-condition is aready fulfilled

To counterargument that, WPF's Dispatcher.InvokeAsync is always async, even when called on the same Dispatcher thread :) It'd be great to have this behavior consistent with WinForms.

Edited, as a compromise, that behavior could be controllable by a flag arg of InvokeAsync, but I'd still make it on by default.

KlausLoeffelmann commented 3 years ago

I am also in favor to have it always async, since this might be the expectation (the somewhat natural "stomach feeling/instinct" if the translation of "Bauchgefühl" makes any sense in English) of your typical WinForms Developer, who did not come in contact with too much async stuff until now.

Although I have to admit, if I wanted to enqueue a call back on the Message Queue, I can hardly think of any scenario, where I wanted to do this to call the Invokee asynchronously. I'd rather force a callback to become event characteristics, but with that, it's rather the normal Invoke you would use.

Edited, as a compromise, that behavior could be controllable by a flag arg of InvokeAsync, but I'd still make it on by default.

I'd rather have a fix, known pattern to check, if Invoke is required, and then leave it to the decision of the dev, if they want to use InvokeAsync or await the call on the same thread directly. I am not sure (again given the fact, that experiences might not be so many due to lack of options in the WinForms world), if too many options/combination are confusing and introduce more options for errors. On top, testing for "Am I on the UI thread?" is a pattern, that most WinForms devs know and use already.

Shidell commented 3 years ago

The recommended practice in WinForms when mutating a Control looks like this:

if (textBox.InvokeRequired)
{
    textBox.BeginInvoke(new MethodInvoker(() =>
    {
        textBox.Text = "Hello World";
    }));

    return;
}

textBox.Text = "Hello World";

But, a lot of developers just skip checking InvokeRequired, and always use BeginInvoke, even if they're already on the UI thread:

textBox.BeginInvoke(new MethodInvoker(() =>
{
    textBox.Text = "Hello World";
}));

4608 looks to improve this by allowing Invoke/BeginInvoke to pass an Action directly, rather than a Delegate, so if accepted, it would look like this:

textBox.BeginInvoke(() =>
{
    textBox.Text = "Hello World";
});

I don't know what the performance implications of checking/not checking if we're already on the UI thread are, but skipping the check and always using BeginInvoke is widely recommended (but, not saying it's necessarily correct), because BeginInvoke is going to PostMessage asynchronously to the Message Queue anyway (as opposed to Invoke, which is using SendMessage synchronously). Part of the reason it's widely suggested is also because it's repetitive boilerplate and calling a method on Control with a Delegate is cumbersome just to mutate a Property (which is why #4608 is being discussed).

I assume that Invoke is used by the framework to handle events internally, like closing a Window, calling to repaint, etc., (events which need to propagate up the Message Queue rapidly and/or be coalesced together), and because of that, it's synchronous nature and the risk synchronicity assumes, that developers should generally not be using it (at least for standard Control mutation purposes)—but I don't know if that's necessarily true or not for certain.

Also, new patterns encourage performing all UI updates on the UI thread (Tasks, Async/Await), but recognizing that it isn't always possible to be on the UI thread (and that exceptions exist), I wanted to suggest that whatever the implementation of InvokeAsync becomes, it should take into consideration and/or handle all of the above concerns:

This is following the comments of @KlausLoeffelmann above: If we're going to provide a new option for WinForms developers, it should incorporate the best practices automatically (include InvokeRequired? Skip it? Always BeginInvoke?)—I think this would go a long way to aide clarity and provide a "this is the way forward" path.

KlausLoeffelmann commented 3 years ago

But, a lot of developers just skip checking InvokeRequired, and always use BeginInvoke, even if they're already on the UI thread:

Not from my experience. What makes you assume this?

but skipping the check and always using BeginInvoke is widely recommended

Who recommends this and why and in what context? Can you point to a bunch of examples?

I assume that Invoke is used by the framework to handle events internally, like closing a Window, calling to repaint, etc.,

Can you elaborate why you assume this? I am having a hard time following you.

Also, new patterns encourage performing all UI updates on the UI thread...

What patterns are you talking about, and where do you get the information for those patterns.

Is there a good reason for developers to be using Invoke (as opposed to BeginInvoke)? Is the recommendation that developers always use BeginInvoke, and Invoke is a framework implementation that is abstracted away and developers don't need to be concerned about it?

I am not sure, if we are - how should I put this - on the same page with our understanding of when to use Invoke and BeginInvoke. Could I ask you the favor to read up on this, so we would be on the same page?

Shidell commented 3 years ago

Not from my experience. What makes you assume this? Who recommends this and why and in what context? Can you point to a bunch of examples?

Microsoft's pages on Invoke and BeginInvoke don't include checking InvokeRequired in their example usage.

The first hits when searching for "Invoke vs. BeginInvoke" on StackOverflow lead to these pages, none of which check:

Your initial proposal doesn't check it, either:

try
{
    ...
    IAsyncResult? asyncResult = BeginInvoke(invokeDelegate, args);
    ...
}

I assume that Invoke is used by the framework to handle events internally, like closing a Window, calling to repaint, etc.,

Can you elaborate why you assume this? I am having a hard time following you.

I believe that SendMessage calls with certain arguments are prioritized internally in the Windows Message Loop, like WM_PAINT and WM_QUIT. IIRC, when receiving some arguments like WM_PAINT, the Message Loop will scan the Message Queue and remove queued messages that are also WM_PAINT.

Invoke is to be used when the caller wants a message to propagate immediately, and will block until it does so—the concern is that in a multithreaded environment, numerous Invokes could cause deadlock on the UI thread.

I guess I was considering InokeAsync as a way of wrapping BeginInvoke into a Task, maybe I'm misunderstanding the usage. Is the suggestion that WinForms developers continue to use Invoke and BeginInvoke, and only use InvokeAsync in conjunction with async controls?

Also, new patterns encourage performing all UI updates on the UI thread...

What patterns are you talking about, and where do you get the information for those patterns.

TAP & Async/Await

Is there a good reason for developers to be using Invoke (as opposed to BeginInvoke)? Is the recommendation that developers always use BeginInvoke, and Invoke is a framework implementation that is abstracted away and developers don't need to be concerned about it?

I am not sure, if we are - how should I put this - on the same page with our understanding of when to use Invoke and BeginInvoke. Could I ask you the favor to read up on this, so we would be on the same page?

Internally, the framework handles events, like closing a Window, right? The immediacy of that is handled by the framework as SendMessage (presumably) and it's handled quickly.

The question is based on whether the standard LOB WinForms application should bother themselves with Invoke when compared to BeginInvoke. BeginInvoke doesn't block, and messages get propagated (eventually), which is generally good enough (but obviously some situations will require Invoke.) Isn't that the point Jon Skeet makes in the first example here?

sharwell commented 3 years ago

The most compelling argument for me on any threading API is this: if your code has a single-threaded UI and involves asynchronous code, it needs to be using Microsoft.VisualStudio.Threading for ease of ensuring correct behavior as the application is developed and maintained. Given that WinForms is a single-threaded UI, defining a new InvokeAsync API here would have exactly one outcome: it would be a new method to add to the list of "do not call" methods for correct asynchronous applications.

kirsan31 commented 3 years ago

@sharwell

The most compelling argument for me on any threading API is this: if your code has a single-threaded UI and involves asynchronous code, it needs to be using Microsoft.VisualStudio.Threading for correctness. Given that WinForms is a single-threaded UI, defining a new InvokeAsync API here would have exactly one outcome: it would be a new method to add to the list of "do not call" methods for correct asynchronous applications.

Did I understand you correctly that you are proposing to completely rewrite WinForms (Invoke, BeginInvoke)?

sharwell commented 3 years ago

Ignoring concerns I have with InvokeAsync in limited scenarios involving external libraries, I have a different set of concerns with the specific API proposal above. The primary open question is whether to invoke the callback synchronously or force a yield when called from the UI thread. Synchronous invocation would allow for simpler coding patterns in common cases, and opens up efficiency improvements.

Proposed alternate signature:

public ValueTask InvokeAsync(
    Func<ValueTask> operation,
    CancellationToken cancellationToken);

public ValueTask InvokeAsync(
    Func<object, ValueTask> operation,
    object state,
    CancellationToken cancellationToken);

public ValueTask<TResult> InvokeAsync<TResult>(
    Func<ValueTask<TResult>> operation,
    CancellationToken cancellationToken);

public ValueTask<TResult> InvokeAsync<TResult>(
    Func<object, ValueTask<TResult>> operation,
    object state,
    CancellationToken cancellationToken);

This method would allow invocation of operation from the UI thread without yielding in a manner that returns a completed ValueTask<T> without allocating.

I didn't include timeOutSpan since it's fairly easy to implement if needed:

using var cancellationTokenSource = new CancellationTokenSource(timeOutSpan);
var result = await control.InvokeAsync(operation, cancellationTokenSource.Token);
KlausLoeffelmann commented 3 years ago

OK, I would like all of us to take a step back, so I can probably also role back my assumption, that the InvokeAsync proposal/semantics is the best solution for a problem I see (no matter how that method may or may not get implemented in the end).

My motivation for the proposal was the following:

What do you do?

My questions more precise:

You are not allowed to add any additional NuGet packages, btw, because you are working on a team with a lot of bureaucracy, and that decision would need to go through a committee, and it would mean you'd miss an important deadline.

noseratio commented 3 years ago

Although I have to admit, if I wanted to enqueue a call back on the Message Queue, I can hardly think of any scenario, where I wanted to do this to call the Invokee asynchronously. I'd rather force a callback to become event characteristics, but with that, it's rather the normal Invoke you would use.

@KlausLoeffelmann, with WinForms it's unfortunately impossible to effectively cancel a queued callback (unlike with WPF's Dispatcher.InvokeAsync - that'd be another useful feature if we could do that). With WPF though, I often used queuing for throttling/debouncing UI updates, as I could easily cancel a previous update without overflowing the UI Thread message queue with pending callback messages.

One other interesting use case for queuing is ShowDialogAsync, which I'm going to propose as a new API for WinForms. There, I used await Task.Yield() to continue on the new dialog's nested message loop, but I could have used InvokeAsync instead.

My questions more precise:

  • What would you (as a WinForms Developer) expect to find as an easy discoverable tool/method/guide to solve the problem?

I'd post a question on StackOverflow and I'd get 10 answers, including one mentioning the new InvokeAsync method, should it already be available 😅

  • What do you, as a WinForms Repo contributor, expect the WinForms Developer to be capable of to solve the problem?

I'd expect them doing the above 🙂

Shidell commented 3 years ago

What do you do?

Your question and scenario is exactly what led me here—I'm a WinForms developer who is trying move legacy code from Events (BackgroundWorkers) into the future via Tasks and Async/Await, and my organization wants to go from WinForms to WinUI 3.0, because they took issue with Silverlight/WPF/UWP for various reasons.

It's for those reasons that I look at Invoke and BeginInvoke, and recognize InvokeRequired, but don't see sources using it—so I question what value it provides. And, the community (StackOverflow, etc.) say, "Just use BeginInvoke, because it's generally all you need, and safe because you post messages asynchronously and don't risk deadlock. It's good enough."

Well, then, why do I need Invoke? ...Do I even need Invoke? What's the best practice? Should I just use BeginInvoke and let Messages bubble up to the UI as the Message Pump can dispatch them, and recognize that it might not always be perfect—but it's better than risking deadlock and/or waiting on a synchronous Invoke call to return?

So, the next logical conclusion is, "I can replace BackgroundWorkers with Tasks, and I'll run multiple Tasks on the Threadpool", and/or "I'll use Parallel.ForEach to accelerate my long-running work—and inside each Task (or inside the Parallel.ForEach loop), I'll update UI components as work finishes, and I'll use Control.BeginInvoke(new MethodInvoker(() => { Control.Text = "Updated"; } )); to make sure I don't involuntarily cause a deadlock."

But, Async/Await exists, and we have to understand it, too—and now what was previously BackgroundWorker(s) is a combination of Async/Await with Tasks and/or Parallel.ForEach, and we need to understand the Task Scheduler, and use Tasks and the TPL with overloads to ensure we run Delegates or Actions on the appropriate UI thread to prevent this problem, and/or wait for the Tasks to complete (or the await to return with UI thread context), and perform all UI updates there.

For example:

BackgroundWorker becomes a Task, Thread, Async/Await, or a Parallel (TPL) operation, and the natural inclination is to update the UI from within the Task, Async method, or each Parallel loop. That requires BeginInvoke.

But (as I understand it), the correct way to go about this is to place work in a Task, Thread, Awaitable or Parallel loop, and have it fire updates outside on the UI thread—or wait for the Await to return me to the UI thread, where I can then perform updates on my own.

So the first thought is:

Task.Run(() => 
{
    Thread.Sleep(1000); // Simulating work

   if (Control.InvokeRequired) Control.BeginInvoke(new MethodInvoker(() => { Control.Text = "Update"; } ));
   else Control.Text = "Update";
});

or:

DoWorkAsync()
{
    Thread.Sleep(1000); // Simulating work

   if (Control.InvokeRequired) Control.BeginInvoke(new MethodInvoker(() => { Control.Text = "Update"; } ));
   else Control.Text = "Update";
}

And then discovering later that this is incorrect, and what we should really be doing is using a callback that's already on the UI thread to post updates, or waiting for an Async method to return, e.g.:

DoWorkAsync()
{
    Thread.Sleep(1000); // Simulating work
}

await DoWorkAsync();
Control.Text = "Update";

And trying to ensure that all UI updates are posted on the UI thread like this. (But, imagining that there are scenarios where it isn't possible), and wondering how it's recommended I go about using InvokeRequired/Invoke/BeginInvoke when I cannot simply mutate a Control on the UI thread.

Which leaves me wondering, why don't all Controls know what their UI thread is? Why do I have to call a Method on a Control, which searches up the Window ownership hierarchy looking for a Handle to locate the Message Pump? Why isn't all of that built into Control, so that regardless of what thread mutates it, it SendMessage(s) or PostMessage(s) appropriately, automatically?

It feels like there are so many pitfalls around ensuring that a Message is enqueued on a Control's Message Queue. Here is an example I faced because of the differences between the SynchronizationContexts, because the default Syncrhonization Context that gets installed in a Console application is different then that of WinForms, and different again of WPF, and I don't know what happens in UWP or WinUI.

All of this struggle, including Delegates and Actions on the UI thread, and Contexts, and capturing/restoring them in Async operations, or using something like await SwitchToMainThreadAsync();, all exists because of marshalling Control updates to it's UI thread.

I'm not blaming anyone (and I'm sure this sounds like ranting), but that's my anecdotal experience so far. I wish Control had the ability to intrinsically manage it's UI thread and/or PostMessage(s) accordingly, and then you can make changes to them from any thread without worry. It's enough to manage Tasks, Threads, Async/Await and Parallel loops properly without extra Actions/Delegates/Contexts to get back to the UI thread and reserve all updates until we're there.

What would you (as a WinForms Developer) expect to find as an easy discoverable tool/method/guide to solve the problem?

A recent document (or guide) authored by Microsoft that shows me "this is the way", with explanations of how (and why), so that I both understand and don't blow my feet off. "How to handle Tasks, Threads, Parallel Loops and Async/Await and why, in 2021".

I read this: https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/async/, and it's helpful, but I'm combining it with articles written by Stephen Cleary and Stephen Toub in 2012 and 2014, and trying to fill in the differences and what's changed by mixing in StackOverflow references when things aren't clear.

What do you, as a WinForms Repo contributor, expect the WinForms Developer to be capable of to solve the problem?

You are not allowed to add any additional NuGet packages, btw, because you are working on a team with a lot of bureaucracy, and that decision would need to go through a committee, and it would mean you'd miss an important deadline.

I think I'm the average WinForms Developer (and maybe it shows), but I think a guide that (again) is a "this is the way" type of document including handling (Events?), Tasks, Threads, Parallelization and Async/Await would go a long way. It doesn't have to cover all the cases (breakouts to covering each topic in depth in other guides would be great), but it could say, "Don't perform UI updates in a BackgroundThread, Task, Thread, or Async method, instead, do it like this ... but if you must (for whatever reason), here's how to do it properly, and here's why."

That's what I would give to leadership and say, "This is how Microsoft recommends we do it."


Ultimately, I just want to do what's correct, and understand why. Tasks, Threads, Async/Await, updating Controls from inside a Task or Async method, or calling Actions or Delegates on the UI thread, or waiting for an Await method to return before updating Controls—whatever the best practice is.

That's why I'm here, I thought InvokeAsync wrapping BeginInvoke in a Task would be the best practice (if accepted), and so as I'm getting my head around everything else, this might be the way to update Controls in 2021.

noseratio commented 3 years ago

Ultimately, I just want to do what's correct, and understand why.

Personally, when I worked with WinForms or WPF, I was following the following pattern, where I very rarely needed BeginInvoke or Invoke:

async Task RunSomeWorkflowOnUIThreadAsync(CancellationToken token) {
    // simulate a CPU-bound work
    await Task.Run(() => 
    {
        for (var i = 0; i < 10; i++)
        {
            token.ThrowIfCancellationRequested();
            Thread.Sleep(100); 
        }
    });

    // update the UI or ViewModel
    control.Text = "computational work done";

    // simulate an IO-bound work
    await Task.Delay(1000, token);

    // update the UI or ViewModel
    control.Text = "I/O work done";   
}

This way, we just stay on the same UI thread through the whole async workflow of RunSomeWorkflowOnUIThreadAsync, and update the UI on that thread.

Of course, we need to account for:

One other approach mentioned in this thread is to use a context-switching mechanisms such as await TaskScheduler.Default.SwitchTo() or await uiThreadTaskScheduler.SwitchTo().

RussKie commented 3 years ago

To be honest I'm having a hard time picturing the scenario for InvokeAsync, and where it would be preferred to the clarity of intent: a) do background work, b) switch to the UI thread to update the UI.

My questions more precise:

  • What would you (as a WinForms Developer) expect to find as an easy discoverable tool/method/guide to solve the problem?
  • What do you, as a WinForms Repo contributor, expect the WinForms Developer to be capable of to solve the problem?

I like several approaches that were mentioned in some shape or form above:

  1. I like declarative approach offered by Microsoft.VisualStudio.Threading and WPF

    private async Task DoWorkAsync()
    {
        // do important work on a background thread
    
        // switch to the UI thread with Microsoft.VisualStudio.Threading and ControlThreadingExtensions.cs
        await this.SwitchToMainThreadAsync();
        // ...or WPF
        // await Dispatcher.SwitchToUi();
    
        // update control properties
        TextTime.Text = DateTime.Now.ToLongTimeString();
    }
  2. I like @kevingosse's idea and be able to write something like

    private async Task DoWorkAsync()
    {
        // do important work on a background thread
    
        // update control properties
        await UpdateControls();
    }
    
    private async UiTask UpdateControls()
    {
        TextTime.Text = DateTime.Now.ToLongTimeString();
    }

Recently as an experiment I have replaced Microsoft.VisualStudio.Threading with the Windows Forms native BeginInvoke in my little app, and it was reasonably straight forward, though in my view lost a bit of expressiveness.

Shidell commented 3 years ago

Are these assertions about Invoke and BeginInvoke correct?

I'm trying to work out if the best practice for all situations where a developer isn't sure if they are on the UI thread, which includes any Event, Thread, Task, or Async/Await method, would be to always mutate via: if (Control.InvokeRequired) Control.BeginInvoke(new MethodInvoker(() => { ... } ));

If the developer is sure they're on the UI thread, then they can mutate as normal, Control.Text = "Update";,

But, if the performance of the Windows Message Pump is fast enough to make a SendMessage vs PostMessage negligible, even in extreme stress situations that aren't realistic, it might be practical to encourage always using BeginInvoke, regardless of the UI thread or not, which would be easier to encourage as a guideline.

(The above assumption includes Async/Await, because an asynchronous Method may or may not actually be running on the UI thread when it's called; it could be called synchronously (for whatever reason, DoJobAsync()), and even if called asynchronously (await DoJobAsync()), the State Machine/Context/Threadpool may decide to run some portion of it on the UI thread. Thus, if a developer needs to mutate a Control from within an Async Method, they should always use if (Control.InvokeRequired) Control.BeginInvoke(new MethodInvoker(() => { ... } ));


Is it possible to modify the Control base class and have it simply handle SendMessage/PostMessage internally, automatically?

public class Control : Component ...
{
    private string _name;
    public string Name
    {
        get
        {
            return _name;
        }

        set
        {
            _name = value;

            if (InvokeRequired)     // Called from non-UI thread
                BeginInvoke(...);   // Use PostMessage to update safely, asynchronously
            else
                Invoke(...);        // UI thread, no worry of deadlock, use SendMessage to update immediately
                                    // (or, perhaps use PostMessage here, too?)
        }
    }
}

If the above is possible, then wouldn't it solve almost all of the problems?

EDIT:

I'm finding conflicting information regarding whether or not Invoke uses SendMessage, or actually uses PostMessage and simply waits for the result before returning. I wish I could examine Control's architecture directly under the hood.

noseratio commented 3 years ago

I'm finding conflicting information regarding whether or not Invoke uses SendMessage, or actually uses PostMessage and simply waits for the result before returning. I wish I could examine Control's architecture directly under the hood.

Not answering other questions, but for this one, it's the latter: PostMessage + a blocking wait. Windows Forms is open-source these days, and you can follow the Control.Invoke logic here.

RussKie commented 3 years ago

It is great to see the excitement and passion everyone has for making Windows Forms more async/await-friendly. However it looks like the discussion is going off the topic, which is requirements/feasibility/necessity for Control.InvokeAsync API.

In the interest of retaining the discussion focused on the subject, may I please ask you to asks unrelated to this topic questions at https://github.com/dotnet/winforms/discussions, and if you have any API proposals (unrelated to this topic) - please start those separately.

Thank you

Eilon commented 3 years ago

We would LOVE to have this as well for Blazor Desktop scenarios. Blazor Desktop supports running Blazor web apps natively in WinForms apps in WebView2. There's a lot of async stuff in WebView2, and Blazor is fully async. When the new BlazorWebView WinForms control is disposed, we need a way to call IAsyncDisposable.DisposeAsync() on its dependencies, but there's no good way to do that in WinForms.

KlausLoeffelmann commented 3 years ago

We're really busy currently with Performance work in the WinForms Designer. Said that, this is still very high on my list - it'll just take a little bit longer to get to it!

Eilon commented 3 years ago

We would gladly be early adopters and try this out once it's available!

KlausLoeffelmann commented 3 years ago

@merriemcgaw for planning and priorities.

UweKeim commented 1 year ago

Will public Task InvokeAsync(Func<Task> invokeDelegate) be available in WinForms for .NET 8?

KlausLoeffelmann commented 1 year ago

It doesn't look too probable currently, to be honest. We need to talk about this internally. @merriemcgaw, @JeremyKuhne FYI.

paul1956 commented 8 months ago

This will also solve the issue with DownloadFile needing to call the new DownloadFileAsync and awaiting the results to be compatible with existing implementation.

jnm2 commented 7 months ago

What do you think about an additional API pattern to cause the remainder of an async method to execute on the correct thread? It's very similar to how await Task.Yield() works, but instead of having a goal of yielding the thread and coming back, the goal would be to switch threads (only if not already on the correct thread).

Instead of writing:

// Maybe not on the UI thread now
await someControl.InvokeAsync(
    async () =>
    {
        // Definitely on the UI thread now
        await AbcAsync();
        await DefAsync();
    });
// Back to maybe not on the UI thread now

It would look like this:

// Maybe not on the UI thread now
await someControl.SwitchToThread();
// Definitely on the UI thread now
await AbcAsync();
await DefAsync();
// Still on the UI thread

Here's an implementation that is very similar in theory, around SynchronizationContext.Post rather than Control.BeginInvoke, but it could be quickly adapted: https://gist.github.com/jnm2/c0ea860c69af1230ac3c0b2d6d010b2a

There's a similar request for .NET itself for the "reverse" operation, moving to a background thread: https://github.com/dotnet/runtime/issues/20025

Continuing to apply this pattern in another place, would you be open to adding await Application.WhenIdle() or similar? I can provide an example implementation for this as well. It would be a custom awaitable like the others, this time using the Application.Idle event to hook up the continuation for the rest of the method.

KlausLoeffelmann commented 7 months ago

That sounds interesting. Give me some time to get my mind behind this!

terrajobst commented 3 months ago

Video

InvokeAsync

namespace System.Windows.Forms;

public partial class Control
{
    public Task InvokeAsync(Action callback, CancellationToken cancellationToken = default);
    public Task<T> InvokeAsync<T>(Func<TResult> callback, CancellationToken cancellationToken = default);
    public Task InvokeAsync(Func<CancellationToken, ValueTask> callback, CancellationToken cancellationToken = default);
    public Task<T> InvokeAsync<T>(Func<CancellationToken, ValueTask<T>> callback, CancellationToken cancellationToken = default);
}

ShowAsync/ShowDialogAsync

namespace System.Windows.Forms;

public partial class TaskDialog
{
    [Experimental("WFOXXXX")]
    public static Task<TaskDialogButton> ShowDialogAsync(nint hwndOwner, TaskDialogPage page, TaskDialogStartupLocation startupLocation = TaskDialogStartupLocation.CenterOwner);

    [Experimental("WFOXXXX")]
    public static Task<TaskDialogButton> ShowDialogAsync(IWin32Window win32Window, TaskDialogPage page, TaskDialogStartupLocation startupLocation = TaskDialogStartupLocation.CenterOwner);

    [Experimental("WFOXXXX")]
    public static Task<TaskDialogButton> ShowDialogAsync(TaskDialogPage page, TaskDialogStartupLocation startupLocation = TaskDialogStartupLocation.CenterOwner);
}

public partial class Form
{
    [Experimental("WFOXXXX")]
    public Task ShowAsync(IWin32Window? owner = null);

    [Experimental("WFOXXXX")]
    public Task<DialogResult> ShowDialogAsync();

    [Experimental("WFOXXXX")]
    public Task<DialogResult> ShowDialogAsync(IWin32Window owner);
}