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

Fix deadlock in DiscordWebhookSink #1018

Merged
merged 1 commit into from
Mar 31, 2024

Conversation

calledude
Copy link
Contributor

Blazor has a SynchronizationContext because it requires rendering to happen on a single thread.
This becomes a problem if an error occurs in the process of rendering, since we have a sink that synchronously blocks because it can no longer return to the thread which is blocked.

This is a solution to the deadlock, however I am open to discussing it.

The general idea I worked with here was to avoid async void as far as possible - which is why I chose to delegate to a separate processor-thread instead.

I also played around with the idea of having it retry multiple times, but not sure whether we want that for the 0.0001% of cases where discord presumably is down (I think that's the only case where an unhandled exception would occur here)

Another solution would be to keep the exact same code, but add ConfigureAwait(false) for all async calls down the chain but I chose to not do this since we would then be relying on an implementation detail.

…ext where a SynchronizationContext exists (e.g. Blazor)
@patrickklaeren patrickklaeren merged commit ff401be into discord-csharp:main Mar 31, 2024
2 checks passed
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