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 WM_DESTROY message handling #12648

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

Sachin-NI
Copy link
Contributor

@Sachin-NI Sachin-NI commented Dec 16, 2024

Fixes #12551

Proposed changes

    • The DetachWindow method always sets Handle and HWND value to NULL. This make code inside if block go in recursive infinite loop. To fix this, we now store the value of Handle in a variable before calling the DetachWindow method. This was the previous behavior but got disrupted due to this change.

I had already completed a PR which stored IsHandleCreated before calling DetachWindow method but missed to store and use Handle value.

Testing

Go through this comment to reproduce this issue. I've validated that this proposed change resolve the issue.
@JeremyKuhne

@Sachin-NI Sachin-NI requested a review from a team as a code owner December 16, 2024 12:08
Copy link

codecov bot commented Dec 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.03283%. Comparing base (a5281d9) to head (86e6bcc).
Report is 6 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                  @@
##                main      #12648          +/-   ##
====================================================
- Coverage   97.03492%   76.03283%   -21.00209%     
====================================================
  Files           1182        3178        +1996     
  Lines         354561      639600      +285039     
  Branches        5411       47215       +41804     
====================================================
+ Hits          344048      486306      +142258     
- Misses          9724      149766      +140042     
- Partials         789        3528        +2739     
Flag Coverage Δ
Debug 76.03283% <100.00000%> (-21.00209%) ⬇️
integration 18.17112% <0.00000%> (?)
production 49.82302% <100.00000%> (?)
test 97.03102% <ø> (-0.00391%) ⬇️
unit 47.04758% <100.00000%> (?)

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

@trj87
Copy link

trj87 commented Dec 17, 2024

Our product which releases early next year requires this fix. Without it, there are several issues including multiple crashes across various features in the product, making it unusable. So it is critical that we get this fix in a patch before our next release and otherwise this would greatly impact our revenue and timelines. Please prioritize this by end of Jan, 2025 or early than that if possible.

Copy link
Member

@Tanya-Solyanik Tanya-Solyanik left a comment

Choose a reason for hiding this comment

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

Thank you!

@Tanya-Solyanik
Copy link
Member

@Sachin-NI - would you be interested in adding a unit test to cover this scenario?

Copy link
Member

@lonitra lonitra left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@lonitra lonitra merged commit af50c4b into dotnet:main Dec 17, 2024
8 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0 Preview1 milestone Dec 17, 2024
Tanya-Solyanik pushed a commit to Tanya-Solyanik/winforms that referenced this pull request Dec 19, 2024
* Fix WM_DESTROY message handling

* modified condition
Epica3055 pushed a commit to Epica3055/winforms that referenced this pull request Dec 19, 2024
* Fix WM_DESTROY message handling

* modified condition
Epica3055 pushed a commit to Epica3055/winforms that referenced this pull request Dec 19, 2024
* Fix WM_DESTROY message handling

* modified condition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.Net 8 upgrade is not able send WM_DESTROY message to ActiveXControl from interop assembly
4 participants