Skip to content

Fix: Plausible Exception while closing an Application#10631

Merged
harshit7962 merged 3 commits intodotnet:mainfrom
harshit7962:fixWindowCloseCrash
Apr 21, 2025
Merged

Fix: Plausible Exception while closing an Application#10631
harshit7962 merged 3 commits intodotnet:mainfrom
harshit7962:fixWindowCloseCrash

Conversation

@harshit7962
Copy link
Member

@harshit7962 harshit7962 commented Mar 24, 2025

Description

The PostMessage call while closing the application might throw an invalid window handle exception. Ideally, the Win32's PostMessage call is not designed to throw an exception; instead, it is meant to return a boolean indicating success or failure. However, the current call is wrapped to throw a Win32Exception. Interestingly, the SetWindowLong call just above this call is in a try-catch block that ignores the invalid window handle exception. It makes sense to move the PostMessage call inside the try-catch block to avoid the same exception being thrown.

The current PostMessage wrapper being used is also modified to get the error code as soon as the failure in IntPostMessage is encountered.

Customer Impact

This might be a potential fix to the issue #7536

Regression

No

Testing

Local Build Pass
Tested with a possible failing scenario

Risk

Low

Microsoft Reviewers: Open in CodeFlow

Copilot AI review requested due to automatic review settings March 24, 2025 17:35
@harshit7962 harshit7962 requested review from a team as code owners March 24, 2025 17:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a plausible exception thrown by the PostMessage call by moving it into the existing try-catch block for SetWindowLong and improving error reporting.

  • Moved the PostMessage call inside the try-catch block in HookUpDefWindowProc to prevent an exception from being thrown on an invalid window handle.
  • Updated the UnsafeNativeMethods PostMessage implementation to capture the Win32 error code before throwing a Win32Exception.
  • Minor header formatting changes in both affected files.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/ManagedWndProcTracker.cs Moved PostMessage call into try-catch block upon non-zero result from SetWindowLong.
src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/UnsafeNativeMethodsCLR.cs Modified PostMessage to capture errorCode via Marshal.GetLastWin32Error() for accurate exceptions.

@dotnet-policy-service dotnet-policy-service bot added the PR metadata: Label to tag PRs, to facilitate with triage label Mar 24, 2025
@codecov
Copy link

codecov bot commented Mar 24, 2025

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 11.26856%. Comparing base (5844add) to head (644b8af).
Report is 84 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #10631         +/-   ##
===================================================
- Coverage   11.39195%   11.26856%   -0.12339%     
===================================================
  Files           3214        3353        +139     
  Lines         648458      668062      +19604     
  Branches       71511       74980       +3469     
===================================================
+ Hits           73872       75281       +1409     
- Misses        573426      591523      +18097     
- Partials        1160        1258         +98     
Flag Coverage Δ
Debug 11.26856% <0.00000%> (-0.12339%) ⬇️

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…dsCLR.cs

Co-authored-by: h3xds1nz <gamingshard@protonmail.com>
@h3xds1nz
Copy link
Member

Anyways, the fix looks reasonable, given that WPF still uses that ugly "convention" of turning win32 errors / NTSTATUSes into exceptions. It's a fun race condition scenario.

@harshit7962 harshit7962 merged commit 93535b0 into dotnet:main Apr 21, 2025
8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators May 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants