Skip to content

[release/7.0] [SignalR] Unblock user callbacks when waiting for client result #44014

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 1 commit into from
Sep 15, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 15, 2022

Backport of #43944 to release/7.0

/cc @halter73 @BrennanConroy

Unblock user callbacks when waiting for client result

This was already an issue with Task returning .On methods, but client results likely makes it more likely to block on the client side

  • .NET client blocks other .On handlers but does not block the receive loop
  • JS client doesn't block anything, .On handlers for future invokes/sends can still run while blocked on user input, pings/etc. still received and processed
  • Java client blocks other .On handlers but does not block the receive loop

Fixes #41996

Customer Impact

@DavidErben writes

Hey guys, I just tried this out with .NET 7 RC1 and the methods are still blocking (like before if one returned a Task). Are there any plans to change this behavior for the official .NET 7 release?

We are using this feature to trigger commands on connected IoT devices. Usually a bunch of commands are triggered simultaneously, so it would be cool if they would run in parallel. But if it is not possible it is not a huge deal either.

Regression?

  • Yes
  • No

No. But without this fix, client return values are not as usable with the C# and Java clients as they are with the TS/JS client.

Risk

  • High
  • Medium
  • Low

This only affects applications taking advantage of SignalR's new client return feature.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@ghost ghost added the area-signalr Includes: SignalR clients and servers label Sep 15, 2022
@halter73
Copy link
Member

@Pilchie This issue was identified a while ago, but we're getting early feedback from rc1 that this is an important fix for .NET 7.

I agree this is really important. It makes the C# and Java clients unusable for certain client return scenarios that work just fine with our TS/JS client. This is a relatively simple change that brings the clients in line with each other.

@Pilchie
Copy link
Member

Pilchie commented Sep 15, 2022

Approved for .NET 7 RC2 (pending passing CI).

@halter73
Copy link
Member

@dotnet/aspnet-build The OSX template failures are unrelated. This PR fully passed in main already. Can we merge this before branching?

@wtgodbe wtgodbe merged commit 5b84d4c into release/7.0 Sep 15, 2022
@wtgodbe
Copy link
Member

wtgodbe commented Sep 15, 2022

That's fine with me

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants