Fix ToolStrip memory leak due to MouseHoverTimer and #4808

kirsan31 commented 4 months ago

Fix #4808 and problem that was mentioned in #11334.

Proposed changes

Replace ToolStripItem? _currentItem with WeakReference<ToolStripItem?> _currentItem in MouseHoverTimer Implement this suggestion.

Customer Impact

No more memory leak if ToolStripItem will be disposed after MouseHoverTimer start. No more memory leak of chilled elements due to DisplayedItems and OverflowItems collections..

Regression?

No

Risk

Minimal.

Screenshots

ToolStripItem below was disposed, but remain in memory: timer1 timer2

Test methodology

Existing unit tests.
New unit tests.
Manual with test app from #4808.
Microsoft Reviewers: Open in CodeFlow

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 98.57143% with 1 line in your changes missing coverage. Please review.

Project coverage is 75.26360%. Comparing base (6b443fc) to head (ccc87f9).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #11358 +/- ## =================================================== + Coverage 75.25676% 75.26360% +0.00683% =================================================== Files 3085 3085 Lines 633571 633629 +58 Branches 46831 46839 +8 =================================================== + Hits 476805 476892 +87 + Misses 153368 153337 -31 - Partials 3398 3400 +2 ``` | [Flag](https://app.codecov.io/gh/dotnet/winforms/pull/11358/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | Coverage Δ | | |---|---|---| | [Debug](https://app.codecov.io/gh/dotnet/winforms/pull/11358/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `75.26360% <98.57143%> (+0.00683%)` | :arrow_up: | | [integration](https://app.codecov.io/gh/dotnet/winforms/pull/11358/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `18.02555% <90.00000%> (+0.01323%)` | :arrow_up: | | [production](https://app.codecov.io/gh/dotnet/winforms/pull/11358/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `48.56822% <95.00000%> (+0.01163%)` | :arrow_up: | | [test](https://app.codecov.io/gh/dotnet/winforms/pull/11358/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `97.02282% <100.00000%> (+0.00041%)` | :arrow_up: | | [unit](https://app.codecov.io/gh/dotnet/winforms/pull/11358/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `45.57567% <95.00000%> (+0.02543%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#carryforward-flags-in-the-pull-request-comment) to find out more.

kirsan31 commented 4 months ago

Added commit that fix #4808 (thanks to @elachlan).

elachlan commented 4 months ago

@Olina-Zhang Could your team please test this pr to confirm the issues are resolved?

Olina-Zhang commented 4 months ago

@Olina-Zhang Could your team please test this pr to confirm the issues are resolved?

@elachlan @kirsan31 Tested this PR, just can confirm that GH issue https://github.com/dotnet/winforms/issues/4808 is fixed, I am not clear how to verify https://github.com/dotnet/winforms/issues/11334, can you give some more information about how to repro it?