-1

Usually when I have await in a scope of a method I need to mark method as async and return Task<>. In a code below compiler allowed me to resolve an async/await action inside a dictionary, I was wondering if it is safe to return void from a method in such case. Please consider a following code:

public void Consume(int ServiceId, MyContext ctx){
  var consumerProvider = new Dictionary<int, Action<SomeClass>>{
    {1, 
       async (consumerContext) => await FirstService.ConsumeAsync(consumerContext)},
    {2, 
       async (consumerContext) => await SecondService.ConsumeAsync(consumerContext)},
  };

  consumerProvider[serviceId](ctx);
}

Again, I am just interested to learn if such wrapped async/await in the dictionary is safe or Consume method should return something else than void, and be marked as async.

I am not interested in refactoring this code not to face this challenge or to improve code quality. I am seeking to deeper understand async/await that is wrapped in some data structure, such as Dictionary.

Mateusz Kopij
  • 93
  • 1
  • 12
  • 5
    Step back, simplify. Just consider assigning one of your lambdas to an `Action`. Realise that you've now got an `async void` lambda and that you should generally avoid those. Wrapping that fact in a dictionary (or anything else) doesn't change things. – Damien_The_Unbeliever Jan 25 '21 at 14:55
  • Your Dictionary should be typed as follows `Dictionary>` – Ackdari Jan 25 '21 at 14:57
  • btw, this implies that `MyContext` "is-a" `SomeClass`, right? – Fildor Jan 25 '21 at 14:58
  • @Damien_The_Unbeliever thank you for your suggestion. Sorry for not being clear enough in my question. I am not trying to avoid the problem. My code example can be reduced not to face async/await problem, but I want to understand how async/await behaves when it is wrapped in a data structure, such as Dictionary. – Mateusz Kopij Jan 25 '21 at 14:59
  • 1
    @Fildor yes it is. Again to me, it is just a code example. I am seeking to understand the behaviour of async/await wrapped in the dictionary. – Mateusz Kopij Jan 25 '21 at 15:01
  • 4
    @MateuszKopij in your case you are building two anonymous async-void methods. The fact that they then are stored in a Dictionary does not change that. So you have just the plain old problem of creating and using async-void method when you don't need to – Ackdari Jan 25 '21 at 15:01
  • @Ackdari it doesn't have to be. I don't want to return anything. Action is good enough. Again, this is not important. The important part in my question is the behaviour of async/await wrapped in the dictionary. – Mateusz Kopij Jan 25 '21 at 15:02
  • The line `consumerProvider[serviceId](ctx);` will fire of whatever `FirstService.ConsumeAsync` or `SecondService.ConsumeAsync` and will continue without waiting for the tasks to complete. So if whatever comes after `consumerProvider[serviceId](ctx);` depends on the completion you will get a bug or runtime error – Ackdari Jan 25 '21 at 15:03
  • 2
    The problem is: there is not really a question in your question. What do you expect to be explained in an answer? If the question is _"What is a correct way to await for dictionary with async action?"_ - there is none. You cannot await this. You need a Func that returns a Task, so it _can_ be awaited. Which by the way would force you to make `Consume` at least `Task Consume(...` or `async Task Consume( ... ` – Fildor Jan 25 '21 at 15:03
  • @Fildor The question I am asking is: Is it safe to wrap async/await in the dictionary as in code above. Or should I always in case of dictionary with async/await mark Consume method with await and force it to return Task<>. You mention that I cannot await it, great. So if I have a code like this with action inside the dictionary is there anything I can do to make it safe to execute? – Mateusz Kopij Jan 25 '21 at 15:13
  • @Fildor is there anything else I can do with this code to make it safe, w/o changing Action to Function? Perhaps there are some patterns. I am sure that designers of async/await thought about it and have some recommendation as what to do in such scenario. Unless the answer is: this code is not safe, don't use it. Always when you have async/await in dictionary you need to return something. Sorry I don't have deep enough understanding to make such call, that's why I am asking. – Mateusz Kopij Jan 25 '21 at 15:16
  • 1
    How do you define "safe to execute"? The code in the question will swallow exceptions. That's the exact problem with `async void` (well, one of...). My recommendation would be to await a Task that's returned from a Func. As it would be with any other `async void` . Like said before: The fact that it is an `async void` is not changed by putting it in a dictionary. It doesn't make it safer in any respect, neither. – Fildor Jan 25 '21 at 15:20
  • If the background of your question is that you need "fire and forget" behavior, then that's a different story. – Fildor Jan 25 '21 at 15:22
  • @Fildor by "safe to execute" I understand that continuation will finish and that I will be awaiting it on some thread. I don't want to the allowed situation when it is possible that an exception thrown by FirstService/SecondService will never be caught by any thread. . Also "safe to execute" means to me that if I see similar code should I always refactor it, or there are other ways of handling it, like some patterns. You see I've never heard anyone saying that you shouldn't wrap async/await action in the dictionary because... you are asking yourself for trouble. – Mateusz Kopij Jan 25 '21 at 15:27
  • for example, I know that I cannot do async/await wrapped in paraller.foreach because it is "not safe" https://stackoverflow.com/questions/11564506/nesting-await-in-parallel-foreach. Is it this same with wrapping it in the dictionary? – Mateusz Kopij Jan 25 '21 at 15:30
  • 1
    That's because you shouldn't have `async void` methods in the first place. (outside event handlers, of course). You probably also don't hear "you shouldn't put an unpinned handgranade into the microwave". :D – Fildor Jan 25 '21 at 15:30
  • `Parallel.ForEach` : yeah, ... I guess that's made explicit because it is not necessarily obvious (especially to newbies, where "newbies" means "new to async/await Task" ). _And_ it's the **combination of the two**. `async void` methods are problematic in and by themselves. Putting them in a Dictionary or List or what have you doesn't improve their problematicness (is that a word?). – Fildor Jan 25 '21 at 15:32
  • @Fildor please consider writing an answer to my questions that it is unsafe just like parallel.ForEach so I can accept it. – Mateusz Kopij Jan 25 '21 at 15:35

1 Answers1

3

Is this code problematic?

- Yes. Because you are basically having an async void method. Which is bad: See Avoid async void

Putting it into a Dictionary or any other datastructure will not improve on this.

Why don't I see people recommend against it more frequently?

In comments, we talked about Parallel.ForEach and that there are a lot of recommendations or warnings to not mix it with async/await.

That's because there, the problem is the combination of the two. Parallel is perfectly fine. Async/await is perfectly fine. But if you mix them, they turn explosive.

async void on the other hand is problematic in and by itself. So you'll probably read a lot of "dude, that method is async void, dont' do that", so that's what you'll be warned about. Not so much the combination with a Dictionary, which in itself is no problem. You could have a Dictionary<int, Func<TContext,Task>> perfectly fine.

Fildor
  • 14,510
  • 4
  • 35
  • 67