Skip to content

Commit 93a2ef9

Browse files
Refactor/Improve InvokeAsync to address potential disposing issues (#13564)
* Refactor/Improve InvokeAsync to address disposing issues of the cancellation token. * Add unit tests for Control.InvokeAsync. * Update docs. * Finalize unit tests. Improve invoke behavior in InvokeAsync. Update folder for Copilot instructions for generating the InvokeAsync unit tests. (Note: They had been manually refactored quite a lot, but still saved tons of time.) * Add review suggestions and improve docs accordingly. * Address review feedback * Fix non-static method.
1 parent 687601b commit 93a2ef9

File tree

6 files changed

+758
-68
lines changed

6 files changed

+758
-68
lines changed
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# InvokeAsync Unit Test Instructions for Copilot
2+
3+
## Method Signatures to Test
4+
5+
```csharp
6+
// Async callback returning ValueTask
7+
public async Task InvokeAsync(Func<CancellationToken, ValueTask> callback, CancellationToken cancellationToken = default)
8+
9+
// Async callback returning ValueTask<T>
10+
public async Task<T> InvokeAsync<T>(Func<CancellationToken, ValueTask<T>> callback, CancellationToken cancellationToken = default)
11+
12+
// Sync callback returning T
13+
public async Task<T> InvokeAsync<T>(Func<T> callback, CancellationToken cancellationToken = default)
14+
15+
// Sync callback returning void
16+
public async Task InvokeAsync(Action callback, CancellationToken cancellationToken = default)
17+
```
18+
19+
## Required Test Coverage
20+
21+
Please create comprehensive unit tests for each overload that verify:
22+
23+
### Core Functionality
24+
- **UI Thread Delegation**: Verify the callback executes on the UI thread (different from calling thread)
25+
- **Cancellation Support**: Test cancellation works even when callback doesn't support it (sync overloads)
26+
- **Async Cancellation**: Test cancellation works when callback supports it (async overloads with CancellationToken)
27+
- **Exception Propagation**: Verify exceptions from callbacks are properly propagated to caller
28+
29+
### Edge Cases
30+
- **Handle Not Created**: Verify `InvalidOperationException` when control handle isn't created
31+
- **Pre-cancelled Token**: Verify early return when token is already cancelled
32+
- **Multiple Concurrent Calls**: Test thread safety with overlapping invocations
33+
- **Reentry Scenarios**: Test calling InvokeAsync from within a callback
34+
35+
### Cancellation Scenarios
36+
- **External Cancellation**: Cancel token while callback is queued/running
37+
- **Callback Cancellation**: For async overloads, test cancellation within the callback itself
38+
- **Registration Cleanup**: Verify cancellation registrations are properly disposed
39+
40+
### Return Value Testing
41+
- **Generic Overloads**: Test proper return value handling for `Task<T>` variants
42+
- **Void Overload**: Test completion signaling for `Action` overload
43+
44+
### Performance/Resource Testing
45+
- **Memory Leaks**: Verify no leaked registrations or task completion sources
46+
- **Async Context**: Verify ConfigureAwait behavior and sync context handling
47+
48+
## Test Structure Guidance
49+
50+
- Use a test control with proper handle creation for UI thread tests
51+
- Use `Thread.CurrentThread.ManagedThreadId` to verify thread marshalling
52+
- Use `CancellationTokenSource` with timeouts for cancellation tests
53+
- Include both immediate and delayed cancellation scenarios
54+
- Test with both short-running and long-running callbacks
55+
- Use appropriate async test patterns with proper awaiting
56+
57+
Create tests that are robust, deterministic, and cover both happy path and error conditions.

Winforms.sln

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,11 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "GDI", "GDI", "{D619FF8C-D99
211211
.github\copilot\GDI\GDIPlus-copilot-instructions.md = .github\copilot\GDI\GDIPlus-copilot-instructions.md
212212
EndProjectSection
213213
EndProject
214+
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Async", "Async", "{7BFA3A16-A19A-4FCE-AE1E-D1187BD92D4C}"
215+
ProjectSection(SolutionItems) = preProject
216+
.github\copilot\Async\invokeAsync_generate_test_instructions.md = .github\copilot\Async\invokeAsync_generate_test_instructions.md
217+
EndProjectSection
218+
EndProject
214219
Global
215220
GlobalSection(SolutionConfigurationPlatforms) = preSolution
216221
Debug|Any CPU = Debug|Any CPU
@@ -1180,6 +1185,7 @@ Global
11801185
{D4D97D78-D213-45DF-B003-9C4C9F2E5E1C} = {8B4B1E09-B3C7-4044-B223-94EDEC1CAA20}
11811186
{442C867C-51C0-8CE5-F067-DF065008E3DA} = {77FEDB47-F7F6-490D-AF7C-ABB4A9E0B9D7}
11821187
{D619FF8C-D99A-48AB-B16B-2F0E819B46D5} = {02EA681E-C7D8-13C7-8484-4AC65E1B71E8}
1188+
{7BFA3A16-A19A-4FCE-AE1E-D1187BD92D4C} = {02EA681E-C7D8-13C7-8484-4AC65E1B71E8}
11831189
EndGlobalSection
11841190
GlobalSection(ExtensibilityGlobals) = postSolution
11851191
SolutionGuid = {7B1B0433-F612-4E5A-BE7E-FCF5B9F6E136}

src/System.Windows.Forms/GlobalSuppressions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
// Publicly shipped API
5+
56
[assembly: SuppressMessage("Naming", "CA1725:Parameter names should match base declaration", Justification = "Public API", Scope = "member", Target = "~M:System.Windows.Forms.ButtonBase.OnKeyDown(System.Windows.Forms.KeyEventArgs)")]
67
[assembly: SuppressMessage("Naming", "CA1725:Parameter names should match base declaration", Justification = "Public API", Scope = "member", Target = "~M:System.Windows.Forms.ButtonBase.OnKeyUp(System.Windows.Forms.KeyEventArgs)")]
78
[assembly: SuppressMessage("Naming", "CA1725:Parameter names should match base declaration", Justification = "Public API", Scope = "member", Target = "~M:System.Windows.Forms.ButtonBase.OnMouseDown(System.Windows.Forms.MouseEventArgs)")]

0 commit comments

Comments
 (0)