Skip to content

[SignalR] Avoid deadlock with closing and user callbacks (#19612) #19664

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

Merged
merged 2 commits into from
Mar 25, 2020

Conversation

BrennanConroy
Copy link
Member

Backport of #19612

Description

If the server closed the client's connection while they were waiting on InvokeAsync in their On(...) handler, the client would deadlock.

Customer Impact

Bug was reported by a customer at #17584
Can cause deadlocks in seemingly innocent code.

As a workaround, customers can refactor their code a bit to trigger InvokeAsync from the On(...) method, but not wait for it inside the On(...) method.

Regression?

No

Risk

Very low, moved a wait during close to fix a deadlock scenario.

@BrennanConroy BrennanConroy added the area-signalr Includes: SignalR clients and servers label Mar 6, 2020
@BrennanConroy BrennanConroy added this to the 3.1.x milestone Mar 6, 2020
@BrennanConroy BrennanConroy requested a review from davidfowl March 6, 2020 23:51
@BrennanConroy BrennanConroy added the Servicing-consider Shiproom approval is required for the issue label Mar 7, 2020
@ghost
Copy link

ghost commented Mar 7, 2020

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

@BrennanConroy BrennanConroy removed the Servicing-consider Shiproom approval is required for the issue label Mar 7, 2020
@BrennanConroy BrennanConroy added the Servicing-consider Shiproom approval is required for the issue label Mar 10, 2020
@ghost
Copy link

ghost commented Mar 10, 2020

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Mar 12, 2020
@leecow leecow modified the milestones: 3.1.x, 3.1.4 Mar 12, 2020
@wtgodbe wtgodbe merged commit 9514a86 into release/3.1 Mar 25, 2020
@wtgodbe wtgodbe deleted the brecon/backport branch March 25, 2020 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants