search

PlayFab / consuldotnet

.NET API for Consul (http://www.consul.io/)
Apache License 2.0
692 stars 193 forks source link

KV Txn call may deadlock #123

Open danielglennross opened 6 years ago

danielglennross commented 6 years ago

Missing CAF here: https://github.com/PlayFab/consuldotnet/blob/master/Consul/KV.cs#L614

Causes deadlock when the call is made in a Sync context (i.e. caller blocks with GetAwaiter().GetResult())

sharkadi-a commented 5 years ago

I have the same issue when running integration tests concurrently. Multiple threads try to get keys using GetAwaiter().GetResult() and deadlock occurs.

sharkadi-a commented 5 years ago

Use Task.Run() and then get Task.Result, this works correctly.

danielglennross commented 5 years ago

True, but this isn't ideal - Using Task.Run, we're just releasing the current thread & offloading the work onto a threadpool thread instead. Ideally, we want the async call: await req.Execute(ct) to return execution without having to synchronize to the original context (via configureAwait(false)) so that we can 'sync over async' in our calling code if needed.

It seems this does happen here: https://github.com/PlayFab/consuldotnet/blob/master/Consul/KV.cs#L409

I'm guessing, the missing CAF on the awaitable in Txn() is not intended