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

Fix ToolStrip memory leak due to MouseHoverTimer and #4808 #11358

Merged
merged 3 commits into from
Sep 16, 2024

Conversation

kirsan31
Copy link
Contributor

@kirsan31 kirsan31 commented May 11, 2024

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

Microsoft Reviewers: Open in CodeFlow

@kirsan31 kirsan31 requested a review from a team as a code owner May 11, 2024 10:12
Copy link

codecov bot commented May 11, 2024

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
@@                 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 Coverage Δ
Debug 75.26360% <98.57143%> (+0.00683%) ⬆️
integration 18.02555% <90.00000%> (+0.01323%) ⬆️
production 48.56822% <95.00000%> (+0.01163%) ⬆️
test 97.02282% <100.00000%> (+0.00041%) ⬆️
unit 45.57567% <95.00000%> (+0.02543%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@kirsan31 kirsan31 changed the title Fix ToolStrip memory leak due to MouseHoverTimer. Fix ToolStrip memory leak due to MouseHoverTimer and #4808 May 12, 2024
@kirsan31
Copy link
Contributor Author

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

@elachlan elachlan added the waiting-review This item is waiting on review by one or more members of team label May 12, 2024
@elachlan
Copy link
Contributor

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

@Olina-Zhang
Copy link
Member

@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 #4808 is fixed, I am not clear how to verify #11334, can you give some more information about how to repro it?

@kirsan31
Copy link
Contributor Author

kirsan31 commented May 14, 2024

@Olina-Zhang

can you give some more information about how to repro it?

Same repro app, the key is to remove item immediately after MouseHoverTimer has started. So simply hover and immediately click:

timer.mp4

image

@Olina-Zhang
Copy link
Member

@kirsan31 thanks for your info.
Adding the GH issue #11334 validation: fixed
image

@elachlan elachlan requested a review from Tanya-Solyanik June 24, 2024 01:41
@lonitra lonitra changed the base branch from main to feature/10.0 July 23, 2024 00:57
@lonitra lonitra changed the base branch from feature/10.0 to main August 15, 2024 01:00
JeremyKuhne
JeremyKuhne previously approved these changes Sep 16, 2024
@JeremyKuhne JeremyKuhne enabled auto-merge (squash) September 16, 2024 18:31
@JeremyKuhne JeremyKuhne merged commit e87b9f2 into dotnet:main Sep 16, 2024
8 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0 Preview1 milestone Sep 16, 2024
@dotnet-policy-service dotnet-policy-service bot removed the waiting-review This item is waiting on review by one or more members of team label Sep 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ToolStripMenuItem can lead to memory leaks
5 participants