TaskCompletionSource and not thread-safe library

188 views Asked by At

I have C# avalonia application uses some not thread-safe library via SDK provided by the developer. More specifically is Windows Zoom SDK. Some SDK functions are built on the event driven pattern. After calling SDK methods, application must wait for the execution result callback arrival. So task-based asynchronous pattern was applied in the application using TaskCompletionSource (please see code below).

After async/await pattern has been applied in the application, the SDK does not work correctly (details). However, this question does not discuss working with the Zoom SDK. The question is about how the use of the async/await pattern potentially can led to incorrect behavior of some not thread-safe library (or SDK)?

SDK wrapper method:

public async Task<bool> SdkMethodAAsync(string parameter)
{
    try
    {
        this.sdkService.SdkMethodA(parameter);

        this.tcs = new TaskCompletionSource<bool>();
        return await this.tcs.Task;
    }
    catch (Exception)
    {
        return false;
    }
    
    return false;
    }

SDK callback handler:

    public void OnMethodAReturn(MethodAResult ret)
    {
        // here some property can also be changed 
        // and which will trigger an event on which SDK calls can be made to         

        this.tcs.TrySetResult(ret == MethodAResult.METHODA_SUCCESS);
    }

High-level code:

    public async Task StartAsync(string parameter1, string parameter2)
    {
        var resultMethodA = await SdkMethodAAsync(parameter1);
        var resultMethodB = await SdkMethodBAsync(parameter2);
    }
1

There are 1 answers

0
JonasH On

The first obvious problem is that you update the task completion source after you have made the method call. The method might complete synchonously, and this will mess up your pattern. Change it to

    this.tcs = new TaskCompletionSource<bool>();
    this.sdkService.SdkMethodA(parameter);        
    return await this.tcs.Task;

Another thing you might want to protect against is concurrent calls. Say that someone tries to do

    var mA = SdkMethodAAsync(parameter1);
    var mB = SdkMethodBAsync(parameter2);
    await Task.WhenAll(mA, mB);

If the SDK does not allow concurrent calls this would be an error, but it is not obvious if the error will be reported, or if it will just result in unexpected behaviors. So you might want to build some mechanism to detect cases like this and handle it in some way.

Another thing that may not be obvious

this.tcs.TrySetResult(ret == MethodAResult.METHODA_SUCCESS);

This will make the task always succeed, with the return value telling if it actually succeeded or not. I would have expected the task to be put in succeeded/failed/canceled state depending on the method result. This might also allow for return value or error messages instead of simply succeeded/failed.