Reputation: 342
I'm looking at some code written a while back that is making me very nervous. The general shape of the methods in questions is like this;
public Task Foo(...){
SyncMethod();
SyncMethod();
...
return AsyncMethod();
}
I realize I can just mark the method async
and do an await
on the last call, but...do I have to? Is this safe to use as is? The sync methods that are called before the last async
method do not have asynchronous alternatives and the Foo method does not care about the result of the AsyncMethod, so it looks like it's ok to just return the awaitable to the caller and let the caller deal with it.
Also, FWIW, the code in question might be called from a thread that's running with an active ASP.NET context, so I'm getting that tingling feeling in my brain that there's some invisible evil that I'm not seeing here?
Upvotes: 2
Views: 607
Reputation: 203826
I realize I can just mark the method
async
and do anawait
on the last call, but...do I have to?
Actually, if you did this, the only change that it would have is that exceptions thrown by either of those synchronous methods would be wrapped into the returned Task
, while in the current implementation the method would throw without ever successfully returning a Task
. The actual effect of what work is done synchronously and what work is done asynchronously is entirely unaffected.
Having said that, both of the options you've mentioned are worrisome. Here you have a method that appears to be asynchronous, meaning someone calling it expects it to return more or less right away, but in reality the method will run synchronously for some amount of time.
If your two synchronous methods are really fast and as a result, you're confident that this very small amount of synchronous work won't be noticeable to any of your callers, then that's fine. If, however, that work will (even potentially) take a non-trivial amount of time to solve, then you have a problem on your hands.
Having actually asynchronous alternatives for those methods would be best, but as a last resort, until you have a better option, often the best you can manage to do is to await Task.Run(() => SyncMethod());
for those methods (which of course means the method now needs to be marked as async
).
Upvotes: 5