Long running synchronous implementation of an interface that returns a Task

Question

I'm using this question as a basis for my question.

---

TL;DR: If you're not supposed to wrap synchronous code in an async wrapper, how do you deal with long-running, thread-blocking methods that implement an interface method which expects an asynchronous implementation?

---

Let's say I have an application that runs continuously to process a work queue. It's a server-side application (running mostly unattended) but it has a UI client to give a more-fine-grained control over the behavior of the application as required by the business processes: start, stop, tweak parameters during execution, get progress, etc.

There's a business logic layer into which services are injected as dependencies.
The BLL defines a set of interfaces for those services.

I want to keep the client responsive: allow UI client to interact with the running process, and I also want threads to be used efficiently because the process needs to be scalable: there could be any number of asynchronous database or disk operations depending on the work in the queue. Thus I'm employing async/await "all the way".

To that end, I have methods in the service interfaces that are obviously designed to encourage async/await and support for cancellation because they take a CancellationToken, are named with "Async", and return Tasks.

I have a data repository service that performs CRUD operations to persist my domain entities. Let's say that at the present time, I'm using an API for this that doesn't natively support async. In the future, I may replace this with one that does, but for the time being the data repository service performs the majority of its operations synchronously, many of them long-running operations (because the API blocks on the database IO).

Now, I understand that methods returning Tasks can run synchronously. The methods in my service class that implement the interfaces in my BLL will run synchronously as I explained, but the consumer (my BLL, client, etc) will assume they are either 1: running asynchronously or 2: running synchronously for a very short time. What the methods shouldn't do is wrap synchronous code inside an async call to Task.Run.

I know I could define both sync and async methods in the interface.
In this case I don't want to do that because of the async "all the way" semantics I'm trying to employ and because I'm not writing an API to be consumed by a customer; as mentioned above, I don't want to change my BLL code later from using the sync version to using the async version.

Here's the data service interface:

public interface IDataRepository
{
    Task<IReadOnlyCollection<Widget>> 
        GetAllWidgetsAsync(CancellationToken cancellationToken);
}

And it's implementation:

public sealed class DataRepository : IDataRepository
{
    public Task<IReadOnlyCollection<Widget>> GetAllWidgetsAsync(
        CancellationToken cancellationToken)
    {
        /******* The idea is that this will 
        /******* all be replaced hopefully soon by an ORM tool. */

        var ret = new List<Widget>();

        // use synchronous API to load records from DB
        var ds = Api.GetSqlServerDataSet(
            "SELECT ID, Name, Description FROM Widgets", DataResources.ConnectionString);

        foreach (DataRow row in ds.Tables[0].Rows)
        {
            cancellationToken.ThrowIfCancellationRequested();
            // build a widget for the row, add to return.  
        }

        // simulate long-running CPU-bound operation.
        DateTime start = DateTime.Now;
        while (DateTime.Now.Subtract(start).TotalSeconds < 10) { }

        return Task.FromResult((IReadOnlyCollection<Widget>) ret.AsReadOnly());
    }
}

The BLL:

public sealed class WorkRunner
{
    private readonly IDataRepository _dataRepository;
    public WorkRunner(IDataRepository dataRepository) => _dataRepository = dataRepository;

    public async Task RunAsync(CancellationToken cancellationToken)
    {
        var allWidgets = await _dataRepository
            .GetAllWidgetsAsync(cancellationToken).ConfigureAwait(false);

        // I'm using Task.Run here because I want this on 
        // another thread even if the above runs synchronously.
        await Task.Run(async () =>
        {
            while (true)
            {
                cancellationToken.ThrowIfCancellationRequested();
                foreach (var widget in allWidgets) { /* do something */ }
                await Task.Delay(2000, cancellationToken); // wait some arbitrary time.
            }
        }).ConfigureAwait(false);
    }
}

Presentation and presentation logic:

private async void HandleStartStopButtonClick(object sender, EventArgs e)
{
    if (!_isRunning)
    {
        await DoStart();
    }
    else
    {
        DoStop();
    }
}

private async Task DoStart()
{
    _isRunning = true;          
    var runner = new WorkRunner(_dependencyContainer.Resolve<IDataRepository>());
    _cancellationTokenSource = new CancellationTokenSource();

    try
    {
        _startStopButton.Text = "Stop";
        _resultsTextBox.Clear();
        await runner.RunAsync(_cancellationTokenSource.Token);
        // set results info in UI (invoking on UI thread).
    }
    catch (OperationCanceledException)
    {
        _resultsTextBox.Text = "Canceled early.";
    }
    catch (Exception ex)
    {
        _resultsTextBox.Text = ex.ToString();
    }
    finally
    {
        _startStopButton.Text = "Start";
    }
}

private void DoStop()
{
    _cancellationTokenSource.Cancel();
    _isRunning = false;
}

So the question is: how do you deal with long-running, blocking methods that implement an interface method which expects an asynchronous implementation? Is this an example where it's preferable to break the "no async wrapper for sync code" rule?

Answer 1

You are not exposing asynchronous wrappers for synchronous methods. You are not the author of the external library, you are the client. As the client, you are adapting the library API to your service interface.

The key reasons for the advice against using asynchronous wrappers for synchronous methods are (summarised from the MSDN article referenced in the question):

1. to ensure the client has knowledge of the true nature of any synchronous library function
2. to give the client control over how to invoke the function (async or sync.)
3. to avoid increasing the surface area of the library by having 2 versions of every function

With respect to your service interface, by defining only async methods you are choosing to invoke the library operations asynchronously no matter what. You are effectively saying, I've made my choice for (2) regardless of (1). And you've given a reasonable reason - long term you know your sync library API will be replaced.

As a side point, even though your external library API functions are synchronous, they are not long-running CPU bound. As you said, they block on IO. They are actually IO-bound. They just block the thread waiting for IO rather than releasing it.