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] Free the tls memory on thread termination #95439

Merged
merged 9 commits into from Nov 30, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 30, 2023

Backport of #95362 to release/8.0

/cc @kunalspathak

Customer Impact

Thread Local Static (TLS) is a new feature we added in .NET 8. The TLS memory that we used to store the TLS data per thread was never freed on thread termination. This results in memory leak. This PR fixes the memory leak issue in .NET 8 as reported by one of our internal partner. It will block them on for their January deployment without this fix.

Testing

CI and verified with the internal partner who reported the issue. With the fix, it doesn't leak memory for them.

Risk

It is less risky because we will now free the thread local memory only during the thread termination of the executing thread.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0. (had to target release/8.0 for quicker turnaround).

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

@JulieLeeMSFT JulieLeeMSFT added the Servicing-consider Issue for next servicing release review label Nov 30, 2023
@JulieLeeMSFT
Copy link
Member

CC @jeffschwMSFT.
CC @carlossanlop.

@JulieLeeMSFT
Copy link
Member

cc @mangod9, @AndyAyersMS.

@JulieLeeMSFT JulieLeeMSFT added this to the 8.0.x milestone Nov 30, 2023
Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping me when Tactics approves it and the CI has no issues, so that I can merge it.

@JulieLeeMSFT JulieLeeMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 30, 2023
@carlossanlop carlossanlop merged commit a6e4834 into release/8.0 Nov 30, 2023
111 of 115 checks passed
@carlossanlop carlossanlop deleted the backport/pr-95362-to-release/8.0 branch November 30, 2023 07:54
@akoeplinger akoeplinger modified the milestones: 8.0.x, 8.0.2 Dec 18, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants