brminnick/asyncawaitbestpractices

Extensions for System.Threading.Tasks.Task and System.Threading.Tasks.ValueTask

asyncawaitbestpractices's Issues

What Dotnet native directives to use with .Net Native tool chain?

Hi, while it's not really an issue of this library, I'm having trouble using it. maybe some of you have experience with using .Net Native tool chain. My question is on stackoverflow:

https://stackoverflow.com/questions/73897432/what-dotnet-native-directives-to-use-for-asyncawaitbestpractices-mvvm

CanExecute issue

Hi.
All details described here:
https://stackoverflow.com/q/59137992/2120193

Async / Await != Multithread

Just saw your talk.
the first explanation of Async and await behavior is a bit wrong / misleading.

see:
https://learn.microsoft.com/en-us/dotnet/csharp/asynchronous-programming/task-asynchronous-programming-model#BKMK_Threads

Simplified:
Async and await cut long work into pieces and each piece is run on the synchronization context (kind of the windows message loop).
This can be multi threaded, but doesn’t have to.

The idea is, at some point down the chain some long running operation is handled by the OS, for example IO or Network. When the OS is finished the result will be available via a message on the message loop(dependening on the synchronization context). At this point the continuation can be run.

If this seems super strange, build a barebones C++ win32 app, in that case you have to actually build the message loop.
If you debug that app you will notice a lot of the time your app is actually doing, …. Nothing?! The app is Idle.

idle is the next concept to understand. Every process has an allotted time. The time from callback into the synchronization context until returning to the OS measured against the allotted time is the percentage your app is taking from the cpu. Depending on the amount of cores, this will be split accordingly. So 4 cores, without multithreading max 25% cpu usage if your app consumes 100% of the allotted time.

Important to understand is that, redrawing the UI should not take 100% of this time, so the remaining time is spend to finish Async methods. Those as well should be short, otherwise what you described with a hanging UI will happen regardless of using or not using Async / await.

I can recommend the long blog post of stehphen toub.

https://devblogs.microsoft.com/dotnet/how-async-await-really-works/

Best

Why is there an in-keyword on a bool parameter

Hi,

trying to learn from your code (as well as from your presentation: liked them a lot :) ) I found things I can't wrap my head around. So maybe you can enlighten me.

Considering The ‘in’-modifier and the readonly structs in C# and of that specifically the 2nd point of the conclusion:

Conclusion

• The readonly structs are very useful from the design and the performance points of view.
• If the size of a readonly struct is bigger than IntPtr.Size you should pass it as an in-parameter for performance reasons.
• You may consider using the in-parameters for reference types to express your intent more clearly.
• You should never use a non-readonly struct as the in parameters because it may negatively affect performance and could lead to an obscure behavior if the struct is mutable.

I got two questions:

1. Why does the in bool continueOnCapturedContext or the in ConfigureAwaitOptions configureAwaitOptions parameter use the in-Keyword? Or better, why do you have overloads with them? Both are passed around as 32bit values, however the IntPtr.Size is at least as large or most likely even 64bit. Furthermore, calling the async method of HandleSafeFireAndForget will copy the value and not the reference anyway.
2. What is the point of using an in-keyword on the in Action<TException>? onException? Yes, the compiler would not let anyone assign a reference to a different action with in the called method. Me calling the method and passing the action, I don't care what is done to the reference once I passed it. And again, all is lost once the HandleSafeFireAndForget-method is called.

SafeFireAndForget is not safe

https://github.com/brminnick/AsyncAwaitBestPractices/blame/main/README.md#L140
This line says that:

`SafeFireAndForget` allows a Task to safely run on a different thread while the calling thread does not wait for its completion.

That couldn't be true, because implementation doesn't switch threads at all

AsyncAwaitBestPractices/Src/AsyncAwaitBestPractices/SafeFireAndForgetExtensions.shared.cs

Lines 77 to 90 in d7ae190

	static async void HandleSafeFireAndForget<TException>(ValueTask valueTask, bool continueOnCapturedContext, Action<TException>? onException) where TException : Exception
	{
	try
	{
	await valueTask.ConfigureAwait(continueOnCapturedContext);
	}
	catch (TException ex) when (_onException != null || onException != null)
	{
	HandleException(ex, onException);
	if (_shouldAlwaysRethrowException)
	throw;
	}
	}

Everything prior first await will be executed on caller thread because that's how C# asyncs work (e.g. F# asyncs are different)

Code below will block caller thread for 5 sec before printing, which demonstrate that SafeFireAndForget isn't actually safe.

static async Task BadTask()
{
    Thread.Sleep(5000);
    await Task.CompletedTask;
}

static void Main(string[] args)
{
    BadTask().SafeFireAndForget();
    Console.WriteLine("Hello World!");
}

Why's MVVMLight's message sent in vain?

Dear @brminnick, after reading #35, I'd like to ask if you'd elaborate on this a bit further?
Because after watching your YT video, I implemented my commands (unifying your awesome IAsyncCommand with customer given requirement of using MVVMLight's RelayCommand) like this:

public IAsyncCommand<string> SaveCommand => _save ?? (_save = new AsyncRelayCommand<string>(async title =>
  {
    await Task.WhenAll(DataService.UpdateTitle(title), CloseDialog(EDITOR_TITLE)).ConfigureAwait(false);
    Messenger.Default.Send(new TitleFinishedMessage(title));
  },
  title => !string.IsNullOrWhiteSpace(title),
  async exception => await ShowError("Title update failed.", exception).ConfigureAwait(false)));

And that's because, using it like

await Task.WhenAll(DataService.UpdateTitle(title), CloseDialog(EDITOR_TITLE, new TitleFinishedMessage(title)))
          .ConfigureAwait(false);

with a centralized Messenger.Default.Send(message) inside CloseDialog() still sends the message, but in vain then.

I've already verified it's not because of using continueOnCapturedContext's value for MVVMLight's constructors keepTargetAlive parameter, too: passing true instead (ignoring potential memory leak for this) still doesn't fix the "message in vain" problem.

Can you help me, understanding this mystery?

Polish UI

Question about capctured context in AsyncCommand

Hi,

wondering why the default is false for continueOnCapturedContext. I mean, I know thats in general the better choice. But most of commands do something with UI, right? For Example Updating a ListView, showing a busy indicator, etc. Im not very deep in Xamarin Forms binding Engine and not sure about thread synchronization in case the AsyncCommand starts something that runs next to the main thread (And yea, theres not always a new thread, but can).

When you think of most common UI cases, is false a good default? Thanks in advance

AsyncAwaitBestPractices/Src/AsyncAwaitBestPractices.MVVM/AsyncCommand.cs

Line 29 in 3157c46

bool continueOnCapturedContext = false)

ArgumentException with Dynamic Handlers

This issue is for .NET Core 3.1 but I believe it would also occur in .NET Framework 4.8 (and probably lower versions).

This issue is for AsyncAwaitBestPractices V4.2.0.

I believe EventManagerService.HandleEvent is throwing exceptions trying to call Invoke on a dynamic delegate that requires DynamicInvoke to be called.

I discovered this in unit tests in my project. I am using FluentAssertions and checking for property changed events to be fired. See the example below:

var vm = new ViewModel();
using var subject = vm.Monitor();
vm.SomeMethod();
subject.Should().RaisePropertyChangeFor(x => x.Value);

I you are not familiar with FluentAssertions, calling vm.Monitor() dynamically creates handlers for all events on the vm object so you can assert behavior of the events.

Here is the stack trace from when the exception occurs.

  Message: 
    System.ArgumentException : MethodInfo must be a runtime MethodInfo object. (Parameter 'this')
  Stack Trace: 
    RTDynamicMethod.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
    EventManagerService.HandleEvent(String& eventName, Object& sender, Object& eventArgs, Dictionary`2& eventHandlers)
    WeakEventManager.RaiseEvent(Object& sender, Object& eventArgs, String& eventName)
    WeakEventManager.RaiseEvent(Object sender, Object eventArgs, String eventName)
    VMBase.OnPropertyChanged(String& propertyName) line 322

In EventManagerService here https://github.com/brminnick/AsyncAwaitBestPractices/blob/master/Src/AsyncAwaitBestPractices/WeakEventManager/EventManagerService.cs#L54 you can see it simply calls Invoke. The correct behavior should be to see if it is a dynamic object and call DynamicInvoke when appropriate.

Looking at the coreclr source here: https://github.com/dotnet/runtime/blob/master/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs#L628 you can see that because it is a dynamic delegate, invoke is not allowed and so this is the source of the exception.

Performance Improvements: Pattern Matching vs If/Else in `AsyncCommand<T>.Execute`

Inspect the IL to compare performance using Pattern Matching vs If/Else

Is "Sealed" necessary for AsyncCommand?

Hi Brandon and thanks in advance.
Do you remember DependentCommand?
The idea is to re-evaluate the CanExecute when one dependent property changes.
It would be nice to have the same thing here...something like DependentAsyncCommand.
But AsyncCommand is a sealed class, it can't be used as a base class and so it means I have to copy-paste to re-implement what is already inside.
Is there a reason?

Parameter change does not invoke CanExecuteChanged

When the command parameter is updated, the CanExecuteChanged-event is not invoked due to missing link to CommandManager.RequerySuggested.
Working implementation could be something like:

        public event EventHandler? CanExecuteChanged
        {
            add => CommandManager.RequerySuggested += value;
            remove => CommandManager.RequerySuggested -= value;
        }

The above code will execute CanExecute() after the command parameter changed, but is only available in WPF projects. Maybe creating a WPF lib could fix that?

Add SourceLink

https://github.com/dotnet/sourcelink

Extend WeakEvent Manager to use Delegate

Add the following APIs to WeakEventManager:

public void AddEventHandler(Delegate handler, [CallerMemberName] string eventName = "");
public void RemoveEventHandler(Delegate handler, [CallerMemberName] string eventName = "");

This allows WeakEventManager to be used with Events that don't use EventHandler, like INotifyPropertyChanged:

class BaseViewModel : INotifyPropertyChanged
{
    readonly WeakEventManager _propertyChangedEventManager = new WeakEventManager();

    event PropertyChangedEventHandler INotifyPropertyChanged.PropertyChanged
    {
        add => _propertyChangedEventManager.AddEventHandler(value);
        remove => _propertyChangedEventManager.RemoveEventHandler(value);
    }

    protected void OnPropertyChanged([CallerMemberName]string propertyName = "") => _propertyChangedEventManager?.HandleEvent(this, new PropertyChangedEventArgs(propertyName), nameof(INotifyPropertyChanged.PropertyChanged));
}

CanExecuteChanged doesn't fire

Hello!
Thank you very much for your solution!
I tried "simple" version from https://johnthiriet.com/mvvm-going-async-with-async-command/#
But I faced a problem that I solved without understanding...
The issue is CanExecuteChanged didn't fire on ObservableCollection change. I tried
collection.CollectionChanged += (sender, e) => CalculateAsync.RaiseCanExecuteChanged();
But nothing happened.
So the only way I found is to remove the following method
public void RaiseCanExecuteChanged() { CanExecuteChanged?.Invoke(this, EventArgs.Empty); }
from your code and replace
public event EventHandler CanExecuteChanged;
with
public event EventHandler CanExecuteChanged { add { CommandManager.RequerySuggested += value; } remove { CommandManager.RequerySuggested -= value; } }
I'm new with all these things, so I don't really understand what I have changed. It works now but seems with kind of delay in releasing the button after a command
If you have a minute, can you please explain the correct way of fixing my problem, please.
Thanks in advance

Cannot use SafeFireAndForget() with ValueTask<T>

SafeFireAndForget() works fine with a normal ValueTask, but not with ValueTask<T>

The error is as follows:

Error (active) CS1929 'ValueTask' does not contain a definition for 'SafeFireAndForget' and the best extension method overload 'SafeFireAndForgetExtensions.SafeFireAndForget(ValueTask, in Action?, in bool)' requires a receiver of type 'System.Threading.Tasks.ValueTask' CCS (net8.0-windows10.0.22621.0)

The workaround of using .AsTask() is not ideal

Error when running from signed assembly

I get the following exception when running from a signed assembly. Can we get your code signed?

TIA.

System.IO.FileLoadException: 'Could not load file or assembly 'AsyncAwaitBestPractices.MVVM, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. A strongly-named assembly is required. (Exception from HRESULT: 0x80131044)'

Execute parameter ignored

Example

// I can't provide argument for async command
public IAsyncCommand Login => new AsyncCommand(async () => 
        {
            await Authenticate(new Credentials(AccessKey, SecretKey));
        }, _ => CanLogin, exception => Notificator.Critical(exception));

// But I can do it for sync version

 public ICommand OpenLink => new DelegateCommand(url => 
        {
            Process.Start(new ProcessStartInfo(url as string));
        });

Fix 1: Func should have object argument and return Task.

Func<Task> _execute;
// Should be 
Func<object, Task> _execute;

Fix 2: ignored parameter for _execute command.

void ICommand.Execute(object parameter)
    {
      // added (parameter)
      this._execute(parameter).SafeFireAndForget(in this._continueOnCapturedContext, in this._onException);
    }

Release-3.0.0, Update HandleEvent Description

For HandleEvent, include Action and EventHandler in the method description that appears in intellisense.

https://github.com/brminnick/AsyncAwaitBestPractices/blob/Release-v3.0.0/Src/AsyncAwaitBestPractices/WeakEventManager/WeakEventManager.cs

WinRT: Windows 8.1 SafeFireAndForget issue "No overload for method"

Hello,

I noticed an error on my WinRT project (Windows 8.1) using your package.
I can't use the SafeFireAndForget extension method because I have this message :
No overload for method 'SafeFireAndForget' takes 1 arguments

Do you have more information about this problem?

Thanks

Nuget package version : 4.1.0

Microsoft Visual Studio Enterprise 2015
Version 14.0.25431.01 Update 3
Microsoft .NET Framework
Version 4.8.03752

Question: Cancellation

Hi,

first of all thanks for a very useful library :-)

My question is: how would I make an AsyncCommand cancellable? Scenario: cmd is bound to a long running operation, say a big download, that the user should be able to cancel via another command.

I can't seem to find any support for cancellation, but maybe I'm overlooking something?

Cheers & thanks,
MR

Does RaiseCanExecuteChanged need to be called on the UI thread?

I found this question at StackOverflow, should the RaiseCanExecuteChanged method be executed on the UI thread instead?

Why people use CommandManager.InvalidateRequerySuggested() on ICommands? - StackOverflow

public void RaiseCanExecuteChanged()
{
    Application.Current.Dispatcher.Invoke(() => CanExecuteChanged?.Invoke(this, null)); 
}

Add NuGet README

https://devblogs.microsoft.com/nuget/add-a-readme-to-your-nuget-package/

CanExecute is called only once

I tried to use the package in the WPF project, but it seems that I missed something.

Let say I have the following command:
SaveQuickEditItemCommand = new AsyncCommand(SaveQuickEditItemCommandExecutedAsync, SaveQuickEditItemCommandCanExecute);

It looks like the method
public bool SaveQuickEditItemCommandCanExecute(object o)
is called only once, when my control is launched.

However, if I use a simple ICommand interface instead of IAsyncCommand it works differently: it is called each time I change the value in my control (or probably even each time the control was redrawn, I'm not sure).

How can I force IAsyncCommand to work the same way?

Possible improvement for explicit ICommand.Execute() implementation in AsyncCommand?

While looking at your awesome example implementation of IAsyncCommand, I noticed that the explicit implementation of ICommand.Execute() over at

AsyncAwaitBestPractices/Src/AsyncAwaitBestPractices.MVVM/AsyncValueCommand.cs

Line 143 in 1678be9

void ICommand.Execute(object parameter) => _execute().SafeFireAndForget(_onException, in _continueOnCapturedContext);

points to _execute() instead of ExecuteAsync() (2 lines above) - out of curiosity: why's that?

Because if the latter would be used, an implementer would have a single point of customization, e. g. for extension of ExecuteAsync() by setting an IsExecuting property via try ... finally (checked inside CanExecute(), of course) to provide for efficient prevention of consecutive calls prior to actually finishing the first execution.

Windows 11 - Smart App Control

Windows 11 introduced Smart App Control. One of the security checks for apps is ensuring that the app and its binaries are signed. Because AsyncAwaitBestPractices does not sign its binaries, software using it will be blocked from running when Smart App Control is enabled.

Please sign DLLs produced for AsyncAwaitBestPractices.

https://support.microsoft.com/en-us/topic/what-is-smart-app-control-285ea03d-fa88-4d56-882e-6698afdb7003

AsyncAwaitBestPractices (unsigned):

Microsoft dll (signed):