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

Added a lot of missing ConfigureAwait(false) statements #101

Merged
merged 2 commits into from
Apr 12, 2019

Conversation

ramonsmits
Copy link
Contributor

Added a lot of missing ConfigureAwait(false) statements, refactored ReturnOnAnyThread -> .ConfigureAwait(false) as that is very confusing for outsiders.

I'm currently experiencing blocking issues due to SmtpServer. It seems to be starving the thread/taskpool.

@cosullivan
Copy link
Owner

Hi, thanks for your PR.

I will accept this because you've done the work to add the missing ConfigureAwait's but on a personal level I disagree about the readability of that method as I think ConfigureAwait(false) is one of the most poorly name method & parameter combinations that I've ever come accros.

With regards to your blocking issues that you are experiencing, do you have a sample that reproduces this? If so I can assist in debugging.

I did previously have a performance test harness that I used to run which would send through emails from the Enron Corpus database. I can try to find that and run it against your sample.

Thanks,
Cain.

@cosullivan cosullivan merged commit 847215d into cosullivan:master Apr 12, 2019
@ramonsmits
Copy link
Contributor Author

@cosullivan The application where I see this happening does this:
Run smtpservice instance that on a message receive publishes the email via mqtt on rabbitmq. When sending a bunch of concurrent emails to it the mqtt stuff results in timeouts. I'm not experiencing this when I just publish the same or many more messages concurrently. At the moment I think it is something in the networking code but as this is a hobby project I don't have much time to research. I'll try to make it as small as possible.

@ramonsmits
Copy link
Contributor Author

By the way, would it be possible to release the current master on nuget?

@ramonsmits ramonsmits deleted the configureawait-false branch April 15, 2019 09:20
@cosullivan
Copy link
Owner

I have pushed a new version from master.

Thanks,
Cain.

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

Successfully merging this pull request may close these issues.

2 participants