Skip to content

[release/5.0] WinRT Reference Tracker memory fixes - #51421

Merged
Anipik merged 7 commits into
dotnet:release/5.0from
AaronRobinsonMSFT:cswinrt_fixes_5_0
May 5, 2021
Merged

[release/5.0] WinRT Reference Tracker memory fixes#51421
Anipik merged 7 commits into
dotnet:release/5.0from
AaronRobinsonMSFT:cswinrt_fixes_5_0

Conversation

@AaronRobinsonMSFT

@AaronRobinsonMSFT AaronRobinsonMSFT commented Apr 16, 2021

Copy link
Copy Markdown
Member

Runtime follow up fixes for .NET 5 - #45622 and microsoft/CsWinRT#782

Description

The following PR is for rare but not uncommon scenarios in the WinUI interactions with .NET. The C#/WinRT has been able to mitigate some of the following issues but they can still occur and results in leaks or a destabilized application environment.

  • Address leak in Reference Tracker runtime (Xaml) requests for helping with circular references.
  • Handle the case where a CCW has been marked for destroy by the GC but a Reference Tracker runtime (Xaml) is in the process of making a QueryInterface.
  • Match pegging order state change of .NET Framework.
  • Add testing for the QueryInterface scenario.

Once this work is in it will be ported to .NET 6.

Customer Impact

Possible crashing behavior when a Reference Tracker runtime (Xaml) performs a QueryInterface on a collected managed object. Memory leak in an uncommon scenario involving breaking circular references using the .NET GC.

Regression

This API is new in .NET 5.0 and these issues have existed since original implementation. However the API is used to address the removal of WinRT built-in support - #37672.

Testing

The C#/WinRT and WinUI teams have signed off on these changes using a private build of the runtime. Additional runtime testing for the fixed behavior in this PR has also been added.

Risk

These changes only impact WinRT scenarios and represent no risk in all other scenarios.

/cc @davidwrighton @Scottj1s @manodasanW @MikeHillberg @jkoritzinsky @elinor-fung

Found during new ComWrappers aggregation API in .NET 6.
It is possible that a Reference Tracker runtime can QI a managed object
wrapper when it has been marked as Destroyed or the associated managed
object has been collected. The QI impl must safely ensure prior to
performing the QI that the GC hasn't collected the wrapped managed object.
ComWrappers API test passes GCStress=0xf

@davidwrighton davidwrighton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

@jeffschwMSFT jeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved. I will take for consideration in 5.0.x

@jeffschwMSFT jeffschwMSFT added this to the 5.0.x milestone Apr 19, 2021
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Apr 20, 2021
@leecow leecow modified the milestones: 5.0.x, 5.0.7 Apr 20, 2021
@Anipik
Anipik merged commit 155406c into dotnet:release/5.0 May 5, 2021
@AaronRobinsonMSFT
AaronRobinsonMSFT deleted the cswinrt_fixes_5_0 branch May 5, 2021 18:49
@ghost ghost locked as resolved and limited conversation to collaborators Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants