2

I have a dictionary with strings as keys, and async functions as the values. It's defined as such:

_messageMap = new Dictionary<string, Func<UpgradeTask, Task>>
{
    { "Upgrade1", Upgrade1 }
};

The functions look like this:

private async Task Upgrade1(UpgradeTask upgradePayload)
{
    await _databaseFunctions.DoUpgrade("Upgrade1", upgradePayload.UpgradeId);
}

This is all contained within a class that has an execute method that will call the appropriate function by the string it gets. It essentially functions as a callback mechanism for when an event happens in the future. Execute looks like this:

public async Task Execute(FutureEvent futureEvent)
{
    var payLoad = JsonSerializer.Deserialize<UpgradeTask>(futureEvent.Message);
    await _messageMap[payLoad.UpgradeId].Invoke(payLoad);
}

This however seems to hang indefinitely if the payload ever had an UpgradeId that's not in the dictionary.

What I expected to have happen is if the UpgradeId exists in the dictionary it will invoke that function. Which works perfectly in that case actually. But what seems to happen if an UpgradeId is in the payload that doesn't exist in the dictionary it hangs indefinitely. It's like it's awaiting something that never happens. I thought it would just skip it or maybe even error out. But it just silently fails and hangs forever. This is a problem because it doesn't actually crash the rest of the app. Everything else seems like it's working fine, but none of the callbacks get processed if there's ever one event that doesn't exist in the dictionary.

Why is this happening? I know I can just check if it exists in the dictionary beforehand. But I'm still very confused on why await just hangs forever at that point. I'd like to understand what I'm doing wrong.

Edit: An example on GitHub: https://github.com/Johnhersh/DictionaryTimerExample This is unfortunately the smallest I could get it. Simplifying it further goes back to giving me the expected behavior of throwing when accessing that key.

John Hershberg
  • 29
  • 2
  • 1
    If the key is not in the dictionary, an exception (KeyNotFoundException) is thrown. Now it depends on how you ultimately call the Execute method, if you always `await` in the call chain or if (and how) you somewhere handle exceptions. – Klaus Gütter Dec 12 '22 at 12:38
  • 4
    It doesn't. Since a dictionary throws a `KeyNotFoundException` when you provide a key that doesn't exist, no invocation is happening. You need to find how/where that exception is being swallowed, and then work out what's happening in the aftermath of that. Maybe this is inside some larger framework that keeps trying to process that same payload over and over again. Impossible to say from the fragments here though – Damien_The_Unbeliever Dec 12 '22 at 12:38
  • 1
    What Damien_The_Unbeliever said. Please provide a [mcve], and we'll all be very eager to find out what is wrong (if it's small enough). – Heinzi Dec 12 '22 at 13:32
  • I tried reproducing this with a simple console app but failed. It actually does error out in most cases. The minimal example ended up being bigger than I'd have liked: https://github.com/Johnhersh/DictionaryTimerExample I cannot seem to remove any more parts from it without it correctly erroring out again. – John Hershberg Dec 12 '22 at 15:50
  • @JohnHershberg: Thank you. Unfortunately, I'm not familiar enough with ASP.NET Core to help here. It might make sense to tag your question as such, to attract people who are. – Heinzi Dec 12 '22 at 21:29
  • 1
    The solution to this ended up being doing this work in a BackgroundService. That way I can do the loop in ExecuteAsync() and it doesn't block and will still error out if something bad happens. – John Hershberg Dec 13 '22 at 11:41

1 Answers1

1

You would need to check if the key exists in the dictionary. Without the checking, the KetNotFoundException is being thrown, which stops executing the method that called the Execute method. It does not hang, it just stops.

So here is the only way to fix it

public async Task Execute(FutureEvent futureEvent)
{
    var payLoad = JsonSerializer.Deserialize<UpgradeTask>(futureEvent.Message);
    if (_messageMap.ContainsKey(payLoad.UpgradeId) {
        await _messageMap[payLoad.UpgradeId].Invoke(payLoad);
    }
}

Or

public async Task Execute(FutureEvent futureEvent)
{
    var payLoad = JsonSerializer.Deserialize<UpgradeTask>(futureEvent.Message);
    if (_messageMap.TryGetValue(payLoad.UpgradeId, out var func) {
        await func.Invoke(payLoad);
    }
}

I would also recommend to return a ValueTask instead of Task to avoid Task memory allocations when the key is not found in the dictionary

theemee
  • 769
  • 2
  • 10
  • 1
    Sorry for downvoting your honest attempt to help, but you seem to have missed the final paragraph of the question: *"I know I can just check if it exists in the dictionary beforehand. But I'm still very confused on why await just hangs forever at that point. I'd like to understand what I'm doing wrong."* Your answer just doesn't answer the question as posed. – Heinzi Dec 12 '22 at 13:53
  • @Heinzi didn't read until the end :) I updated the answer to clarify the behavior that the author is experiencing – theemee Dec 12 '22 at 13:59
  • Thanks, but I still don't see how that explains it: When an unhandled exception occurs in .NET, the default behavior of the runtime is to crash the application, not to hang (or "stop", as you call it). – Heinzi Dec 12 '22 at 15:03
  • @Heinzi if he does not await this task (and he probably does not), then the exception will not be thrown in the caller method – theemee Dec 12 '22 at 15:34
  • I appreciate the help! As far as I can see everything is awaited. I have a minimal example up. What I noticed while creating this example is that it's got several layers of async stuff. There's a Timer that calls these. And I'm guessing somewhere down the line something gets messed up and put into a separate thread and the error happens on that thread and doesn't get reported back. But I can't quite isolate where. – John Hershberg Dec 12 '22 at 16:57
  • @JohnHershberg you do not await the `private StartTimer` in the `public StartAsync` method. That is why it does not throw the exception to the main thread. – theemee Dec 13 '22 at 09:50
  • @JohnHershberg you could change the StartAsync method to `public async void StartAsync` and `await _timerTask` inside of it. This way you would get the exception thrown to the main thread. But beware, any uncaught exception inside `async void` will 100% crash the entire program. And it is not possible to catch the exception outside of `async void`, only inside of it. `async void` is considered a bad practice as it can lead to serious bugs. I hope you understood why it does not throw (you do not await StartTimer task) – theemee Dec 13 '22 at 10:09
  • @theemee So if I understand it correctly, because the timer task is not awaited it still happens but on a separate thread or something? Then when an exception is thrown there's no way for it to communicate with the main thread? – John Hershberg Dec 13 '22 at 11:03
  • @JohnHershberg the expection is being thrown, but it is saved in the Task object until you explicitly await it and then it is being thrown. As you can see you are indeed awaiting the _timeTask in the StopAsync method. But it is called after app.Run(). So if you use Ctrl+C in the terminal which will stop app.Run(), the StopAsync method will be called and you will see the KeyNotFoundException being thrown – theemee Dec 13 '22 at 11:15
  • @theemee Very much appreciated, I understand the problem better now. – John Hershberg Dec 13 '22 at 11:23