search

microsoft / kiota-abstractions-dotnet

Abstractions library for the Kiota generated SDKs in dotnet
https://aka.ms/kiota/docs
MIT License
24 stars 21 forks source link

Avoid using GetAwaiter().GetResult() to prevent deadlocks #226

Closed MihaMarkic closed 1 month ago

MihaMarkic commented 2 months ago

Getting synchronously results from async functions is quite an issue and should be avoided. Specially in a UI thread environments as described in Stephen's blog post. Sample code in KiotaSerializer.GetStreamFromString method:

// Some clients enforce async stream processing.
writer.WriteAsync(source).GetAwaiter().GetResult();

I guess once async overloads are added (#223), this code could be made pure sync and when async is required, a dedicated function can be used instead. Albeit it might cause breaking changes.

baywet commented 2 months ago

Thanks for flagging this! I'd go a step further, let's make it pure sync once we have the async overrides, and mark it as obsolete to see what kind of feedback we get. Thoughts @andrueastman ?

MihaMarkic commented 2 months ago

Even better, IMO

andrueastman commented 2 months ago

No objections from my side. The functions are already not async so making them purely sync is what we should do here really.

MihaMarkic commented 1 month ago

Deserialization is taken care with #225

baywet commented 1 month ago

Thanks for the update. I guess at this point the only missing part would be to change that to the sync overload. https://github.com/microsoft/kiota-abstractions-dotnet/blob/56efcdf4227baef8ee75adc342f06e23e96ab3fb/src/serialization/KiotaSerializer.Serialization.cs#L104

Would you mind sending one last PR our way to wrap this aspect up?

MihaMarkic commented 1 month ago

@baywet Sure, created. Do you need also version bump?