Improper use of Task .Result when making async methods sync

[magkal]

Hi! We've ran into issues with our ThreadPool that was caused by the non async methods on the IConfigCatClient interface called too often in our application. Below are references to the lines in the client that calls .Result and needs to be refactored. The use of .Result on an async method is very dangerous and produces deadlocks. It should be refactored to either .GetAwaiter().GetResult() or or something like this snippet, from some years ago, (microsofts entity framework team used this to get async methods run sync).

public static class AsyncUtility
{
private static readonly TaskFactory _myTaskFactory = new TaskFactory(
    CancellationToken.None,
    TaskCreationOptions.None,
    TaskContinuationOptions.None,
    TaskScheduler.Default);

public static TResult RunSync<TResult>(Func<Task<TResult>> func)
{
    return _myTaskFactory
            .StartNew<Task<TResult>>(func)
            .Unwrap<TResult>()
            .GetAwaiter()
            .GetResult();
}

public static void RunSync(Func<Task> func)
{
    _myTaskFactory
            .StartNew<Task>(func)
            .Unwrap()
            .GetAwaiter()
            .GetResult();
}
}

https://github.com/configcat/.net-sdk/blob/7bd21b9620aaaf1175743140bd067c01ea694af3/src/ConfigCatClient/ConfigCatClient.cs#L146

https://github.com/configcat/.net-sdk/blob/7bd21b9620aaaf1175743140bd067c01ea694af3/src/ConfigCatClient/ConfigCatClient.cs#L180

https://github.com/configcat/.net-sdk/blob/7bd21b9620aaaf1175743140bd067c01ea694af3/src/ConfigCatClient/ConfigCatClient.cs#L233

https://github.com/configcat/.net-sdk/blob/7bd21b9620aaaf1175743140bd067c01ea694af3/src/ConfigCatClient/ConfigCatClient.cs#L267

[endret]

hi @magkal , Thank you the report. We're starting to investigate of your issue.

Regards, Endre

[endret]

Hi @magkal,

I think we should split this issue into two parts:

1. Thread synchronization deadlock Have got ever any deadlock when using ConfigCat client?
2. Thread pool issue (i guess you had thread pool starvation) Can you explain what does it mean 'too often'? Why are you using in this case synchronous method instead of async? Is it possible to insert a copy of some lines of code about configcat usage?

I create some changes to replace an elegant .Result / .Wait but it won't solve your ThreadPool issues. (https://github.com/configcat/.net-sdk/pull/18)

Thanks, Endre

[magkal]

Hi thanks for the update, You're right it's two separate issues.

1. We were unable to get our azure webapp starting after deploy and did some debugging with dnSpy, which always got stuck on the configcat library causing a deadlock. As soon as we refactored the code calling Sync methods in configcat, to properly await the result, our azure webapp started and was healthy.

2. Regarding the thread pooling is likely something else that we mixed up in this. Since we upgraded multiple packages within the same test release we did, and an issue we ran into seemed to be caused by thread pool starvation. I think it's safe to disregard that within this github issue.

/Magnus

[endret]

Hi @magkal,

The new version was released.

If you have some time, can you try the async version of the methods?

Thanks, Endre