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

Is it deadlock safe ? #5

Closed
Twodio opened this issue Jul 23, 2019 · 3 comments
Closed

Is it deadlock safe ? #5

Twodio opened this issue Jul 23, 2019 · 3 comments

Comments

@Twodio
Copy link

Twodio commented Jul 23, 2019

Hi again, last week i've forgot to ask you if there were deadlocks or any other async/sync tests for your async methods within sync methods.

In your example, line 36, you're calling the method MainAsync within a sync method, but i also read that the console's Main method may be an exception to these cases where you can actually call an async method and get fine with it in most cases without causing the application to crash.

MainAsync(args).GetAwaiter().GetResult();

The fact that this code will run synchronously may be a problem in most of the cases, after some reading on "don't block asynchronous code". As it looks, it may be there just for demonstration purposes and no real application at all, but i couldn't avoid to look at the same call in Retry.cs, line 34.

HandleExceptionAsync(retryOptions, ex, retryCount).GetAwaiter().GetResult();

After some reading and research about this method, i've found some warnings like "they're meant to be used by the compiler" and "they may cause deadlock".

All these approaches seems more like a way to unwrap the exceptions being thrown if any. My big question is: Is it safe to use GetAwaiter().GetResult() in this context ?

@petrhaus
Copy link
Collaborator

Good catch. There are a few corner cases on which it could cause deadlocks, although I'm running it in a few high concurrency backend services without any (apparent) issue.
Anyway better avoid it and run in the threadpool via Task.Run() and then wait for the execution.
I'll try to update it asap (or feel free to open a pull if you have time).
Thanks.

@Twodio
Copy link
Author

Twodio commented Jul 23, 2019

You're such a great person, always answering in record time. I already handled this issue using Task.Run() in my examples, but as i already said i didn't have the chance to test your code even a bit, or mine fully. My code is now very different from yours since i only picked the main concept of yours(repository base) and from there everything changed. If you don't get the chance to update it today or during this week, i'll make the effort to send a pull request during these days or in the weekend if possible. I also noticed that you were asking for more examples, if i get the chance, i'll open a pull request for that aswell. Anyways your code looks geat man and helped me with my intern struggle on finding a better way to deal with such a concept, thanks for sharing that.

@petrhaus
Copy link
Collaborator

Thanks a lot for the nice words, I appreciate a lot your feedback and I'm glad the code has been of some help for you.
I'll update the code this week and close the issue once done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants