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/6.0] Backport FileStream finalization fixes #80439

Merged
merged 4 commits into from
Jan 11, 2023

Conversation

adamsitnik
Copy link
Member

@adamsitnik adamsitnik commented Jan 10, 2023

Backport of #64997 and #65899 to release/6.0

Customer Impact

Multiple users reported performance issues (#78553, #64741) caused by a lack of proper finalization suppression of the file stream buffering strategy. Users who create a lot of FileStream instances (with buffering enabled, which is the default and can't not be controlled application-wide with a single flag) and dispose them properly suffer from having plenty of finalizable objects in the finalizer queue. It leads to performance degradation.

Testing

Automated tests were added and they are passing. Additional local validation confirmed the leak is gone.

Risk

The risk is low. The fixes "matured" in main and were released as part of .NET 7 (last fix merged 10 months ago) .

fixes #78553

…net#65899)

* add missing GC.SuppressFinalize(this) to FileStream.DisposeAsync

* DerivedFileStreamStrategy.DisposeAsync does not need to call _fileStream.BaseDisposeAsync, as FileStream.DisposeAsync calls Dispose(false) now
@ghost
Copy link

ghost commented Jan 10, 2023

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #64997 and #65899 to release/6.0

fixes #78553

Author: adamsitnik
Assignees: -
Labels:

area-System.IO

Milestone: -

@adamsitnik adamsitnik added this to the 6.0.x milestone Jan 10, 2023
@adamsitnik adamsitnik added the Servicing-consider Issue for next servicing release review label Jan 10, 2023
Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

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

Looks good to me; thanks, @adamsitnik. We will await Tactics review.

@carlossanlop carlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 11, 2023
@carlossanlop carlossanlop modified the milestones: 6.0.x, 6.0.14 Jan 11, 2023
@carlossanlop
Copy link
Member

Approved by Tactics via email (6.0.14).
Signed off by area owner.
No OOB changes needed for System.IO.FileSystem or System.Private.CoreLib.
CI failures unrelated: #80252 and #80476
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit b00e7e9 into dotnet:release/6.0 Jan 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants