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

Synchronous functions causes deadlocks #50

Closed
avanveelen opened this issue Dec 12, 2019 · 2 comments
Closed

Synchronous functions causes deadlocks #50

avanveelen opened this issue Dec 12, 2019 · 2 comments
Assignees

Comments

@avanveelen
Copy link

avanveelen commented Dec 12, 2019

This issue is the reason of the behaviour in #44 .

The synchronous versions of the public methods (e.g. EncryptStream) are just calls to the async version of that method (e.g. EncryptStreamAsync) suffixed with .Wait().

Example:
EncryptStreamAsync(inputStream, outputStream, publicKeyStreams, armor, withIntegrityCheck).Wait();

You should never do this because it causes deadlocks, let me try to explain why:

Wait will synchronously block until the task completes. So the current thread is literally blocked waiting for the task to complete. When the code reaches an await statement, it returns an uncompleted task. When this Tasks completes, it wants to synchronize with the request context from where the Task is initiated. It will wait until this thread comes available. Because the Wait() method blocks the current thread, it will never become available: deadlock.

As a general rule, you should use "async all the way down"; that is, don't block on async code.
What you should do is make the synchronous methods truly synchronous, or, remove them so the calling clients are forced to use the async methods.

There is another option though, but it is more or less a hack: suffix every async call with ConfigureAwait(false). This will prevent the task from synchronizing back with the current request context, avoiding a deadlock.

Here is a good read about this topic.

@mattosaurus
Copy link
Owner

Thanks for pointing that out, I didn't realise Wait was quite that bad. I'll probably depreciate the sync rather than converting them with the aim of removing them further down the road.

@mattosaurus mattosaurus self-assigned this Dec 13, 2019
@mattosaurus
Copy link
Owner

Fixed in release v2.2.0

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