Handle disposed AsyncWaitHandle during BeginInvoke completion - #14998
Conversation
|
Thanks for the excellent report and focused fix. |
KlausLoeffelmann
left a comment
There was a problem hiding this comment.
LGTM!
Thanks for your contribution!
There was a problem hiding this comment.
🟢 Approval recommended
Pull request overview
This PR hardens WinForms Control.BeginInvoke completion so that a consumer-disposed IAsyncResult.AsyncWaitHandle no longer causes an ObjectDisposedException on the UI thread during marshaled callback completion.
Changes:
- Catch and tolerate
ObjectDisposedExceptionwhenThreadMethodEntry.Complete()signals the internalManualResetEvent. - Add a regression test ensuring the marshaled callback still runs and the returned
IAsyncResultreports completion even ifAsyncWaitHandleis disposed.
File summaries
| File | Description |
|---|---|
| src/System.Windows.Forms/System/Windows/Forms/Control.ThreadMethodEntry.cs | Wraps completion signaling (_resetEvent?.Set()) in an ObjectDisposedException handler to prevent UI-thread crashes when the wait handle is externally disposed. |
| src/test/unit/System.Windows.Forms/System/Windows/Forms/ControlTests.Methods.cs | Adds a regression test that disposes AsyncWaitHandle before invoking marshaled callbacks and verifies callback execution and IsCompleted. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| { | ||
| _resetEvent?.Set(); | ||
| } | ||
| catch (ObjectDisposedException) |
There was a problem hiding this comment.
Instead of catching an exception, can we check: if(!_marshaler.IsDisposed)
There was a problem hiding this comment.
No. We could have the waitHandle disposed ...
IAsyncResult result = control.BeginInvoke(callback);
result.AsyncWaitHandle.Dispose();
... but not the marshaling control.
|
Merging this, as this is a bug fix of an existing feature, and not a contribution of a new feature. @jaywang-cn - for contribution other than bug fixes, please take a look at the contributor licence agreement, which we would need to take respective PRs. Thanks for you help on this matter! |
Summary
ObjectDisposedExceptionwhenThreadMethodEntry.Completesignals anAsyncWaitHandledisposed by its consumerFixes #14996
Testing
dotnet test src/test/unit/System.Windows.Forms/System.Windows.Forms.Tests.csproj --no-restore -- --filter-method System.Windows.Forms.Tests.ControlTests.Control_BeginInvoke_DisposedAsyncWaitHandle_CompletesCallback(1 passed)Microsoft Reviewers: Open in CodeFlow