Skip to content

Fix CloseMessage issues #34294

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

Closed
BrennanConroy opened this issue Jul 12, 2021 · 2 comments
Closed

Fix CloseMessage issues #34294

BrennanConroy opened this issue Jul 12, 2021 · 2 comments
Labels
area-signalr Includes: SignalR clients and servers enhancement This issue represents an ask for new feature or an enhancement to an existing one

Comments

@BrennanConroy
Copy link
Member

The main issue is that CloseMessage is written to the pipe and then the app task exits, then connection cleanup starts which will complete the transport pipes, this results in a race where the CloseMessage might not get sent if there was any backpressure or thread scheduling was unfavorable for the pipe dispatches.

Additionally, there are scenarios where we would like to specify the reason for closing the connection and it is non-trivial to do that, so we should make it easier.

Potential solutions:
Wait a little after sending the close message to give it time to send
Somehow detect that the close message is sent before completing the app task (with timeout of course)

@BrennanConroy BrennanConroy added the area-signalr Includes: SignalR clients and servers label Jul 12, 2021
@BrennanConroy BrennanConroy added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Jul 14, 2021
@BrennanConroy BrennanConroy added this to the Backlog milestone Jul 14, 2021
@ghost
Copy link

ghost commented Jul 14, 2021

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@BrennanConroy
Copy link
Member Author

Looks like this was fixed in #25693 just forgot about it when opening this issue.

@BrennanConroy BrennanConroy removed this from the Backlog milestone May 30, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-signalr Includes: SignalR clients and servers enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests

1 participant