3

Let's say I'm trying to write a library that abstracts certain actions. In this example I want to turn a light on or off. There could be hundreds of different kinds of lights that are controlled in lots of different ways, so I have an interface like so:

public interface ILight
{
    bool TurnOnLight();
    bool TurnOffLight();
}

Or like:

public interface ILight
{
    Task<bool> TurnOnLight();
    Task<bool> TurnOffLight();
}

The design of the interface is that calling the method should turn on/off the light and then return a boolean saying if it was successful or not. The amount of time this takes is indeterminate.

The desire/goal is to have a common interface that developers who are unfamiliar with the hardware can use. They'd just call TurnOnLight and get a return if it worked or not, regardless of the implementation.

The issue is that, in the implementation of the interface, some of the implementations involve async operations, and others do not. These "restrictions" exist in the various 3rd party libraries used to communicate with the lights. Whether or not these libraries are performing "real" async operations is unknown, just that they return an awaitable task with results.

Library1.TurnOnLight();

vs

await Library2.TurnOnLight();

The developer using this stuff shouldn't have to worry about what type of light is being used, just that they want to turn it on or off. That makes me think I'd end up with an interface and implementation like:

public interface ILight
{
    Task<bool> TurnOnLight();
    Task<bool> TurnOffLight();
}

public class Light1 : ILight
{
    public Task<bool> TurnOnlight()
    {
        bool result = Library1.TurnOnLight();
        return Task.FromResult(result);
    }
}

public class Light2 : ILight
{
    public async Task<bool> TurnOnlight()
    {
        return await Library2.TurnOnLight();
    }
}

I've seen some people reference using a similar design pattern, and others who say it's bad practice to have something return a task that is synchronous. I lean towards the implementation I've got here, are there gotchas or issues with this design that I should be aware of?

2

To point out the English semantics: "can await things" is not synonymous with "will await things".

Just because an async method is able to return control to the owning thread for the time being (i.e. while awaiting something), doesn't mean that it invariably will do so.
As a basic example, if your code starts a task, performs some other work, and then awaits that task, it's possible that the task already finished by the time your code awaits it, which means that your code will never actually wait for the task to complete, it will immediately continue with the rest of its work.

Your ILight hits on this precise distinction. It enforces that any light which implements the interface must make their method awaitable. However, the interface does not in fact enforce that any light which implements the interface must actually await something.

Your approach is perfectly fine.

I've seen some people [..] say it's bad practice to have something return a task that is synchronous.

That statement does not account for cases like your example, where you're trying to define a blanket interface that accounts for both synchronous and asynchronous implementations.

If none of the implementations make use of await, be it in the method body or a submethod below it, then using async is indeed irrelevant.

Although even then you could still argue that introducing async is the first step of upgrading the library's contract to being async-friendly, before actually rewriting the method bodies themselves.

0

Yes, there are some gotchas, indeed!

Before all, let's agree on something, it is not a good idea to expose asynchronous wrappers for synchronous methods (quite authoritative source).

Long story short, regarding why:

  • The simplest of reasons is that you are pretty much spawning a new Task, burdening the current Thread pool and this is something that your consumer just might not want. If all you do is Task.Run(...), then there is nothing stopping your consumer from doing just that, wherever they deem absolutely necessary (usually close to the UI, which is where async makes all the difference).

  • Careless usage of your methods (e.g. synchronously waiting with .Wait() or .Result) may lead to deadlocks (though you can hack your way using ConfigureAwait(false) wherever possible...see Avoiding Context and Preventing the Deadlock and make absolutely sure you understand why this works).

  • You could expose asynchronous interfaces when you are dealing with pure/naturally asynchronous methods (like, e.g. I/O... generally, methods that are not CPU-bound). The idea behind that is that CPU resources are precious and scarce, therefore, you should not make decisions on behalf of your consumer).

Now, regarding your situation, it seems you may be out of luck, since 3rd party libraries are asynchronous anyway. Based on how deep knowledge you have about this library, if you are certain that their methods are naturally asynchronous (and implemented as such), you can definitely offer asynchronous abstractions. The general advice that I could give you, at least when you are stuck (e.g. with 3rd party libraries or code you have no control over), is to avoid lying about the true underlying implementations. You just cannot abstract async efficiently, it runs all the way down/up.

I am afraid I cannot offer more suggestions (note that I am avoiding the word advice as I am not much of an expert on all things async) unless you further elaborate.

EDIT/UPDATE:

Based on your comment, what I am suggesting is not to provide two flavors of everything (a synchronous and an asynchronous counterpart), but to provide only one of everything, based on what the library call is: use async when abstracting a method that is already async or you know is naturally asynchronous, otherwise avoid asynchronous wrappers, keep everything else synchronous so that the decision can be taken further up the "call stack", whether the called method should be wrapped in asynchronous methods or not.

If you want to hide your implementations behind an interface and have some of them be async and some others not async, I am afraid that your only option what you are proposing.

This answer pretty much covers your case (#1 in specific). Some of your implementations are asynchronous, while others are not! So, for what it's worth, your proposed design is your best bet. It also allows you to change your synchronous implementations to truly asynchronous in the future.

7
  • All good advice (I've read several of the articles you linked before), but I feel like this is more of generic guidelines of when to use async, and less about the specific situation. The interface implementation(s) are like plugins for interfacing with different hardware, and no, I don't know enough about the dozens of vendor specific libraries to know how "true" their async is. Are you suggesting that there be twice as many functions, for both asyc and not, and that the developer should know enough about the individual devices to use them differently? That's what I was trying to avoid.
    – cost
    Apr 3 '20 at 22:22
  • @cost I have updated my answer. But you seem to imply that you need a common interface, implementations of which may or may not be asynchronous, is that right? Apr 3 '20 at 22:29
  • Yes, I need/want a common interface so the developer can just call TurnLightOn and not worry about any other details. I'll update my question too. Some, but not all of the internal implementations, use libraries that have async calls to return if it was a success or not. Because of that, I have to have Task/await all the way up so that the returned bool can be passed up. Other libraries just return a bool.
    – cost
    Apr 3 '20 at 22:33
  • About your most recent edit with the link, that was one place I found that makes that recommendation. I haven't been able to find too much dialogue about it though, which is why I was looking for extra thoughts here. Thanks for your input!
    – cost
    Apr 3 '20 at 22:40
  • I have updated my answer again. Yes, based on your requirements, I think your proposed design is the most advantageous at the moment. Maybe we can wait to see what others have to propose. Apr 3 '20 at 22:40
0

I understand my previous answer was not very helpful. Let me try again...

There are naturally asynchronous operations, and operations that are not naturally asynchronous.

public async Task TurnTheLightsOn()
{
    await _light.TurnOnLight();
}

MSDN informs us

The async and await keywords don't cause additional threads to be created. Async methods don't require multithreading because an async method doesn't run on its own thread. The method runs on the current synchronization context and uses time on the thread only when the method is active. [...]

If no additional threads are created, how is asynchrony "simulated"? It's right in there: ...an async method doesn't run on its own thread. That is, of course, unless it has to.

When you provide a Task returning method, you may be spawning a new thread, or you may have a naturally asynchronous operation, but a message that you do send is that your call is non-blocking. See this answer for an example of something going wrong because of this misunderstanding.

Your Task<bool>-returning method that exposes the synchronous TurnOnLight() library method as a seemingly asynchronous operation is practically a trick. So, going back to the previous consideration, this method really doesn't run on its own thread (as per MSDN). Which thread does it run on, then? That's right, it runs on the calling thread. If you await _light.TurnOnLight() and the fake-Task method is called, regardless of the await, the call is blocking, because the only "exit-point" in your implementation is the return Task.FromResult<bool>(...) line of code.

So, if your TurnOnLight() method is CPU-bound (fancy term for "takes quite a bit of cpu time to complete", alternatively a "long-running" method), calling it in a context where non-blocking calls are a requirement (e.g. to have a responsive GUI) can lead to funny code. Any developer finding that out will end up doing something along the lines (well, OK, almost... just vary to taste):

public async Task SwitchTheLightsOn()
{
    //Update UI element notifying the user that the light is being turned on.
    //Also update UI indicator of "work-being-done".
    //...

    bool lightsOn = await Task<bool>.Run(() =>
    {
        //This call is blocking, for some reason, so it is necessary to
        //send it to the "background" for execution, otherwise, the UI
        //doesn't update while waiting...
        return _light.TurnOnLight();
    });
    
    //Update UI elements back to the visual indication of an idle state.
    //Inform the UI about success or failure using the lightsOn variable.
    //...
}

In short, make sure that your TurnOnLight() method is not CPU-bound. Otherwise, you should find a way to indicate that this method is long-running.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.