microsoft / PowerPlatform-DataverseServiceClient

Code Replica for Microsoft.PowerPlatform.Dataverse.Client and supporting nuget packages.
MIT License
277 stars 50 forks source link

Azure Web App Alert: Mixing synchronous code with asynchronous code #291

Closed TW-JCP closed 2 years ago

TW-JCP commented 2 years ago

We are using the DataverseServiceClient in an Azure App Service and have received an email alert from Azure App Service analytics saying that Microsoft.PowerPlatform.Dataverse.Client.ConnectionService.GetCachedService "is calling Task.Result and is synchronously waiting on the Task object, which can lead to deadlocks in the process ... Please review the call stack closely and make sure that your code is using the C# await keyword."

The alert references the Microsoft document "Async All the Way" to "ensure that the code is following recommendations to avoid deadlocks and to improve app performance."

Below is the relevant portions of the call stack in the alert:

System.Threading.ManualResetEventSlim.Wait System.Threading.Tasks.Task.SpinThenBlockingWait System.Threading.Tasks.Task.InternalWait System.Threading.Tasks.Task`1[[System.__Canon mscorlib]].GetResultCore Microsoft.PowerPlatform.Dataverse.Client.ConnectionService.GetCachedService Microsoft.PowerPlatform.Dataverse.Client.ConnectionService.IntilizeService Microsoft.PowerPlatform.Dataverse.Client.ServiceClient.CreateServiceConnection Microsoft.PowerPlatform.Dataverse.Client.ServiceClient..ctor

Line 951 of Microsoft.PowerPlatform.Dataverse.Client.ConnectionService does indeed appear to be calling .Result on a task: IOrganizationService localSvc = InitServiceAsync().Result;

From a few discussion threads and other issues, I know Async is still a work-in-progress, that the API is still mostly sync and that various patterns are needed depending on the situation, but figured it wouldn't hurt to post the alert. I'm sure we won't be the only ones to see this alert.

MattB-msft commented 2 years ago

@TW-JCP , thanks for the post, Yes we are aware of this and its an unfortunate by product of a few different challenges we are trying to reconcile without creating a significant breaking behavior in customer upgrade paths.

Currently the plan is to deal with this by adding a "connectasync()' behavior that would be used in conjunction with the deferred connection behavior we added in 1.0 .

This will be added a bit later as we work though some other issues with it.

thanks MattB