Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process will terminate when the async event throw any exception #76367

Closed
lindexi opened this issue Sep 29, 2022 · 10 comments
Closed

Process will terminate when the async event throw any exception #76367

lindexi opened this issue Sep 29, 2022 · 10 comments

Comments

@lindexi
Copy link
Member

lindexi commented Sep 29, 2022

Description

Some code add the event and work with asynchronous logic. But I find the process will terminate when the async event throw any exception.

I think it's a scary question. This means that the process will crash at any time and we can't handle exceptions in a unified code.

Reproduction Steps

Raise the FooEvent event. And you will find the Process be terminated.

                FooEvent += async (sender, args) =>
                {
                    await Task.Delay(1);
                    // some code throw exception
                    throw new Exception();
                };

Expected behavior

Maybe we can ignore the exception

Actual behavior

The exception will terminate.

Do we have to write try-catch in every event to improve stability?

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 29, 2022
@stephentoub
Copy link
Member

Unhandled exceptions crash the process, by design. It's the same for exceptions going unhandled in queued thread pool work items, for example. With async Task methods, the exception is stored into the returned task for it to be propagated to wherever that Task is awaited. With async void, there's nowhere to store an unhandled exception, so it's queued to be rethrown on whatever SynchronizationContext was current when the method was invoked. This enables any centralized exception mechanism for the current environment (e.g. WinForms) to handle it.

@stephentoub stephentoub closed this as not planned Won't fix, can't repro, duplicate, stale Sep 29, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Sep 29, 2022
@lindexi
Copy link
Member Author

lindexi commented Sep 30, 2022

@stephentoub Thank you. Can I ask the best practice about handle async event exception?

Should try-catch be used for all async events? I'm worried that some unexpected exception occurs in an unimportant module, causing the process to exit. Or some code executed in the plugin also encountered this problem, causing the process to exit.

@stephentoub
Copy link
Member

The first question to ask yourself is if you really need an async void method at all? If the answer is "yes", then the question to ask is what would you want to have happen if an unexpected exception occurred? What happens in the case of a synchronous event handler for the same event throwing an exception, and how does the resulting behavior in environment in question differ when the exception is thrown asynchronously? If the behavior of allowing the exception to go unhandled is unacceptable, then your recourse is to catch the exception and handle it instead in whatever manner makes the most sense.

@lindexi
Copy link
Member Author

lindexi commented Sep 30, 2022

Thank you @stephentoub

@lindexi
Copy link
Member Author

lindexi commented Oct 10, 2022

@stephentoub I have a question: If there are some unexpected exceptions in async event, such as OutOfMemoryException, that the process will terminate. It means that the OutOfMemoryException may crash the application when we do not try-catch in async event handler. I'm worried this will make the dotnet application unstable. Is my idea reasonable? Thank you.

@stephentoub
Copy link
Member

That's no different from an OutOfMemoryException happening in a queued work item (e.g. ThreadPool.QueueUserWorkItem) or a new Thread (e.g. new Thread(...)), etc. Additionally, if you get an unexpected exception in the middle of doing something, that thing will have been interrupted at an unexpected time, so the state of your application is already likely inconsistent making it possibly unstable.

@Clockwork-Muse
Copy link
Contributor

It means that the OutOfMemoryException may crash the application

You're almost never supposed to catch OoM anyways, because your ability to handle and recover from it are pretty low.

I'm worried this will make the dotnet application unstable.

Recovery in the face of exceptions - even the "foreseeable" ones you're supposed to handle (mostly IO related) - is a non-trivial task at the best of times.

@lindexi
Copy link
Member Author

lindexi commented Oct 10, 2022

Thank you @stephentoub and @Clockwork-Muse . My understanding is that if I can handle exceptions well, such as OutOfMemoryException , then I should catch and handle it. Is my understanding correct?

@Clockwork-Muse
Copy link
Contributor

such as OutOfMemoryException

Outside of some very specific scenarios - which are unlikely to be relevant here - you shouldn't be catching "unrecoverable" exceptions, which are usually:

  • OoM or StackOverflowException; these represent resource exhaustion, and generally your process won't have the overhead available to even catch and log them in the first place.
  • Anything that's a subclass of ArgumentException. These represent programmer errors, and there isn't a way for your program to automatically recover from them.
  • InvalidOperationException, for similar reasons.

In general, the exceptions you're supposed to handle and recover from are "foreseeable". They're most commonly:

  • A subclass of IOException, or anything dealing with files or remote connections (eg, HttpRequestException). Because these regularly do occur in normal usage.
  • OperationCanceledException/TaskCanceledException
  • Anything resulting from dealing directly with user input. Eg, FormatException. In most cases the exceptions can be worked around because these methods often have TryWhatever() variants that don't throw the "recoverable" errors.

Normally any given try ... catch should only try to deal with one or two categories, scoped as tightly as possible to be recoverable.
Note that it's possible that whatever you're doing should actually be recovered at a higher level, which would mean you'd need to change the signature to be async Task or something, instead of the async void it is currently. That is, you wouldn't have the try ... catch inside the task/method, but outside it or at some other point (error handlers in web servers often work this way, for example).


All this aside, though, we don't have enough information about what you're actually doing to give better advice (and this sort of isn't the place for it, as opposed to something like StackOverflow).

@ghost ghost locked as resolved and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants