Avoid calling Task.Result and Task.Run

[nathan-alden-sr]

It is well known that one should never call Task.Result, Task.Wait(), or Task.WaitAll() in normal code due to the possibility of deadlocks. I see a couple of places where you are calling the Task.Result property. You should rewrite those places like this: someTask.ConfigureAwait(false).GetAwaiter().GetResult(). This is the idiomatic way to run a task when inside a method that is not asynchronous.

There is also no need to call Task.Run unless you intend to either await the call or to fire-and-forget. If you need the result and you're inside a synchronous method, you should write the code as I've done above.

[Insomniak47]

@nathan-alden-sr question: not sure exactly how GetAwaiter.GetResult prevents deadlocks. My understanding of the only differences were that .Result throws an AggregateException if the task throws and GetResult() will throw the actual exception. For my benefit do you have any docs that explain this one a bit better?

[Dean-McMillan]

This has caught us out in our Azure function app in the GetBytes() method - deadlocks killed the processing. Is there a suggested way to avoid this? Currently looking at this as a way to avoid it via a Task.Run:

        var t = Task.Run(() => argon2.GetBytes(16));
        t.Wait();
        return t.Result;

OR... as most suggest, make sure that there is no sync/async code, that is making the Method that calls the library thus:

public static Task<byte[]> HashPassword(string password, byte[] salt)

[kmaragon]

The intention was to have people use the Async contracts. After several years in the wild, it's clear that that intention is either impractical for most porjects or is just being ignored. I've got some time to work on this for a few weeks. So expect a 2.0 release that just gets rid of any ability to do this async but will resolve these issues.