BuildConfigFromConfigFile() hangs

[aloneguid]

The static methods on KubernetesClientConfiguration like BuildConfigFromFile are hanging when using from certain environments where SynchronisationContext is not default. For instance WPF, Xamarin etc. Specifically on any line that uses .GetAwaiter().GetResult();.

Could you provide true async methods where you are waiting for operation result?

[brendandburns]

Just for clarity, you're looking for something like:

Task<KubernetesConfiguration> BuildConfigFromFileAsync(...)

[aloneguid]

Correct. Going further, in my opinion there should be no code relying on .GetResult() as it's potentially deadlocking. If users of this library want to be synchronous they are always welcome to introduce locking themselves.

[brendandburns]

I added the async methods. Since the sync methods are part of the existing contract, it's a breaking change to remove them, so I kept them as well. Seems like a decent balance.

[aloneguid]

Thank you, looks good :)

[brendandburns]

Can you put an LGTM on the PR? Then we can get it merged.

Thanks!

[lolodi]

Thanks for your timely fix I was able to get unblocked, but looking at the rest of the code it looks like the default behavior is to use .GetAwaiter().GetResult(); everywhere, even in the generated code.
This can create deadlocks if the consumer has its own SynchronizationContext as pointed out by @aloneguid. As I was reading here this is not recommended and .ConfigureAwait(false) should be used to avoid the default SynchronizationContext capturing behavior. I found an interesting article here that elaborates more about the whole SynchronizationContext.
Would it be possible to change the library to expose real Async API for every action as well as using .ConfigureAwait(false) for the forcefully synchronized ones?

[brendandburns]

The generated code is generated by the AutoRest tool: https://github.com/Azure/autorest The fixes would need to come from there.

…

--brendan

________________________________ From: Lorenzo <notifications@github.com> Sent: Tuesday, February 11, 2020 3:48 PM To: kubernetes-client/csharp <csharp@noreply.github.com> Cc: Brendan Burns <bburns@microsoft.com>; Comment <comment@noreply.github.com> Subject: Re: [kubernetes-client/csharp] BuildConfigFromConfigFile() hangs (#352) Thanks for your timely fix I was able to get unblocked but looking at the rest of the code it looks like the default behavior is to use .GetAwaiter().GetResult(); everywhere, even in the generated code. This can create deadlocks if the consumer has its own SynchronizationContext as pointed out by @aloneguid<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Faloneguid&data=02%7C01%7Cbburns%40microsoft.com%7C30e4114de4c24cfd53cb08d7af4cdfc9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637170617038052742&sdata=DQCPdoRV6lHRlNex56CDIc0P2H3UpAFpxJ4rHT8PUAE%3D&reserved=0>. As I was reading here<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmedium.com%2Fbynder-tech%2Fc-why-you-should-use-configureawait-false-in-your-library-code-d7837dce3d7f&data=02%7C01%7Cbburns%40microsoft.com%7C30e4114de4c24cfd53cb08d7af4cdfc9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637170617038052742&sdata=A0erRuPM54kznjFZ379iHy6VvTeOmwlpH%2FzuEcsnJMM%3D&reserved=0> this is not recommended and .ConfigureAwait(false) should be used before the .GetAwaiter().GetResult(); to avoid the default SynchronizationContext capturing behavior. I found an interesting article here<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Farchive%2Fmsdn-magazine%2F2011%2Ffebruary%2Fmsdn-magazine-parallel-computing-it-s-all-about-the-synchronizationcontext&data=02%7C01%7Cbburns%40microsoft.com%7C30e4114de4c24cfd53cb08d7af4cdfc9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637170617038062744&sdata=IWS7A09MADzxXLiyji%2F8y1LUR9bE8Q%2FSVJmqjvBHvlE%3D&reserved=0> that elaborates more about the whole SynchronizationContext. Would it be possible to change the library to expose real Async API for every action as well as using .ConfigureAwait(false) for the forcefully synchronized ones? — You are receiving this because you commented. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkubernetes-client%2Fcsharp%2Fissues%2F352%3Femail_source%3Dnotifications%26email_token%3DABL4HATQHHSCBUNMUMZHTRLRCM2MFA5CNFSM4KNSVGF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELOTHYY%23issuecomment-584922083&data=02%7C01%7Cbburns%40microsoft.com%7C30e4114de4c24cfd53cb08d7af4cdfc9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637170617038062744&sdata=Ow02b6W8y5QbxYvvJdOFQIrncwWoQVR9hT1dxdYJZas%3D&reserved=0>, or unsubscribe<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABL4HAVIFFPZVI3XMA6BOA3RCM2MFANCNFSM4KNSVGFQ&data=02%7C01%7Cbburns%40microsoft.com%7C30e4114de4c24cfd53cb08d7af4cdfc9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637170617038072731&sdata=6%2FyMumNvHbEqKRkVosRD3FuigWlVxpFS%2FqvLp6hhh58%3D&reserved=0>.

[lolodi]

I see. But even without the AutoRest changes, it is possible to avoid binding on the caller SyncronizationContext and expose more async APIs. There is an interesting article https://devblogs.microsoft.com/dotnet/configureawait-faq/, running the underlying async operation in a Task.Run().GetAwaiter().GetResult() should be enough to guarantee that the operation is executed in a background task withouth a SyncronizationContext.