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

[release/8.0] Dispose CTS in HubConnection streaming #51137

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 4, 2023

Backport of #51117 to release/8.0

/cc @BrennanConroy

Dispose CTS in HubConnection streaming

Description

Fixes a memory leak in the SignalR client when using streaming.

Customer Impact

Noticed by customer when running a service with a client for multiple weeks and seeing large memory usage. Could also be seen by a more active client that makes a lot of streaming calls.

Regression?

  • Yes
  • No

Looks like it's been there since 3.X

Risk

  • High
  • Medium
  • Low

Simple disposal of an object once it's done being used.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-signalr Includes: SignalR clients and servers label Oct 4, 2023
@BrennanConroy BrennanConroy added the Servicing-consider Shiproom approval is required for the issue label Oct 4, 2023
@ghost
Copy link

ghost commented Oct 4, 2023

Hi @github-actions[bot]. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@BrennanConroy BrennanConroy added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Oct 4, 2023
@ghost
Copy link

ghost commented Oct 4, 2023

Hi @github-actions[bot]. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed.

@ghost
Copy link

ghost commented Oct 13, 2023

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@ghost ghost added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Oct 13, 2023
@BrennanConroy
Copy link
Member

/azp run

@ghost ghost removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Oct 13, 2023
@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@BrennanConroy BrennanConroy added this to the 8.0.0 milestone Oct 16, 2023
@BrennanConroy
Copy link
Member

@dotnet/aspnet-build can this be merged?

@wtgodbe wtgodbe merged commit 0c61054 into release/8.0 Oct 18, 2023
25 checks passed
@wtgodbe wtgodbe deleted the backport/pr-51117-to-release/8.0 branch October 18, 2023 18:33
@ghost ghost modified the milestone: 8.0.0 Oct 18, 2023
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework 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.

2 participants