Add async/await support for all long-running DISM operations#2
Add async/await support for all long-running DISM operations#2
Conversation
|
Please add xUnit tests for all the new async methods you added to this PR. Add them to the existing test project at src/Microsoft.Dism.Tests/. Follow the exact same patterns and conventions as the existing tests — use DismTestBase or TestBase as the base class, use xUnit ([Fact]), and match the file/class naming convention (e.g. AddPackageAsyncTest.cs). The tests are integration tests using the install.wim file already in the project. Cover at minimum: (1) happy path — task completes successfully, (2) cancellation — already-cancelled CancellationToken throws OperationCanceledException, (3) progress — IProgress gets called. Do NOT open a new PR — commit directly to this branch. |
Rolling2405
left a comment
There was a problem hiding this comment.
Please add xUnit tests for all the new async methods you added to this PR. Add them to the existing test project at src/Microsoft.Dism.Tests/. Follow the exact same patterns and conventions as the existing tests — use DismTestBase or TestBase as the base class, use xUnit ([Fact]), and match the file/class naming convention (e.g. AddPackageAsyncTest.cs). The tests are integration tests using the install.wim file already in the project. Cover at minimum: (1) happy path — task completes successfully, (2) cancellation — already-cancelled CancellationToken throws OperationCanceledException, (3) progress — IProgress gets called. Do NOT open a new PR — commit directly to this branch.
Added 12 test files with 33 total test methods in commit
Also fixed a cancellation bug where
Done in commit |
46ac78e to
41eafc9
Compare
--- updated-dependencies: - dependency-name: Microsoft.SourceLink.GitHub dependency-version: 10.0.201 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…braryImport Added net10.0 to TargetFrameworks with AllowUnsafeBlocks for source-generated P/Invoke - Converted all 48 DllImport declarations to LibraryImport with #if NET7_0_OR_GREATER conditional compilation - Added [UnmanagedFunctionPointer] to native callback delegate - Moved ArtifactsPath to Directory.Build.props (NETSDK1199 fix for .NET 10) - All 4 targets build successfully: net40, netstandard2.0, net8.0, net10.0
- SA1116: Collapse multi-line DismProgress constructor to single line - SA1116: Move first param of Task.Factory.StartNew to its own line - SA1117: Split closing params of Task.Factory.StartNew onto separate lines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Rolling2405 <89894749+Rolling2405@users.noreply.github.com>
… blocks Reindent lambda body and trailing parameters in all 12 async method files so that the opening brace, body, and remaining arguments are consistently indented at 16 spaces (one level deeper than the method call), satisfying SA1137 (same indentation) and SA1117 (each parameter on its own line). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Rolling2405 <89894749+Rolling2405@users.noreply.github.com>
the private sync helper methods in 4 files to satisfy SA1202
('public' members should come before 'private' members):
- DismApi.EnableFeature.cs
- DismApi.MountImage.cs
- DismApi.RemovePackage.cs
- DismApi.SetEdition.cs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Rolling2405 <89894749+Rolling2405@users.noreply.github.com>
…RS0016/RS0017 for net40 - Remove optional parameter defaults from MountImageAsync overloads to fix RS0026 - Add all 17 public async method signatures to PublicAPI.Unshipped.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Rolling2405 <89894749+Rolling2405@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Rolling2405/ManagedDism/sessions/d5267b48-44f8-4364-ac85-dab68ef56967 Co-authored-by: Rolling2405 <89894749+Rolling2405@users.noreply.github.com>
49e1448 to
2c2c360
Compare
The DISM API is entirely synchronous, blocking the calling thread during long-running native operations. This causes UI freezes in WPF/WinForms/MAUI apps, and
Task.Runalone doesn't solve it because the native progress callback fires on the calling thread.Adds
*Asynccounterparts for all 17 public methods that acceptDismProgressCallback, using theTaskCompletionSourcepattern recommended by the maintainer.Implementation
TaskCompletionSource<bool>+Task.Factory.StartNewwithTaskCreationOptions.LongRunning— notTask.Run, since native DISM calls are truly long-running blocking operationsIProgress<DismProgress>instead ofDismProgressCallback(standard .NET async pattern)CancellationTokenwired toDismProgress.Cancel = true, which signals the nativeEventWaitHandle.CancellationToken.Noneis passed toTask.Factory.StartNew(not the caller's token) to ensure the delegate always runs — cancellation is handled inside the delegate to avoid hangingtcs.Taskwhen the token is already cancelled.#if !NET40sinceIProgress<T>requires .NET 4.5+Methods added
DismApi.AddPackage.csAddPackageAsyncDismApi.AddCapability.csAddCapabilityAsyncDismApi.CheckImageHealth.csCheckImageHealthAsync→Task<DismImageHealthState>DismApi.CommitImage.csCommitImageAsyncDismApi.DisableFeature.csDisableFeatureAsyncDismApi.EnableFeature.csEnableFeatureAsync,EnableFeatureByPackageNameAsync,EnableFeatureByPackagePathAsyncDismApi.MountImage.csMountImageAsync(by index),MountImageAsync(by name)DismApi.RemoveCapability.csRemoveCapabilityAsyncDismApi.RemovePackage.csRemovePackageByNameAsync,RemovePackageByPathAsyncDismApi.RestoreImageHealth.csRestoreImageHealthAsyncDismApi.SetEdition.csSetEditionAsync,SetEditionAndProductKeyAsyncDismApi.UnmountImage.csUnmountImageAsyncTests added
12 test files (33 total test methods) in
src/Microsoft.Dism.Tests/, following existing conventions (DismTestBase/DismInstallWimTestBase, xUnit[Fact], Shouldly assertions). Tests are guarded with#if !NETFRAMEWORKsince the test project's net472 target resolves to the main library's net40 build where async methods are excluded.CheckImageHealthAsyncTest.csAddPackageAsyncTest.csAddCapabilityAsyncTest.csCommitImageAsyncTest.csDisableFeatureAsyncTest.csEnableFeatureAsyncTest.csMountImageAsyncTest.csRemoveCapabilityAsyncTest.csRemovePackageAsyncTest.csRestoreImageHealthAsyncTest.csSetEditionAsyncTest.csUnmountImageAsyncTest.csUsage
Builds clean (0 warnings, 0 errors) across all four targets:
net40,netstandard2.0,net8.0,net10.0. Purely additive — no existing APIs or behavior modified.Original prompt
Add async/await support for all long-running DISM operations (Issue jeffkl#82)
Summary
Add async/await versions of all long-running DISM API methods that currently accept a
DismProgressCallback. This addresses Issue jeffkl#82 in the upstream repo (jeffkl/ManagedDism), which is labeledhelp wantedandenhancement.Background
The current API is entirely synchronous. Every long-running method (e.g.
AddPackage,EnableFeature,MountImage, etc.) blocks the calling thread while the native DISM operation runs. This causes UI freezes in WPF/WinForms/MAUI apps.Maintainer Guidance
The maintainer @jeffkl specifically recommended using
TaskCompletionSource<T>for this implementation (NOTTask.Run). From Issue jeffkl#82:Reference: https://devblogs.microsoft.com/pfxteam/the-nature-of-taskcompletionsourcetresult/
Additionally, a real user @TheJoeFin reported that simply wrapping calls in
Task.Rundid NOT work for their WPF app — the UI still locked up. This is likely because the native DISM progress callback fires on the calling thread, soTask.Runalone doesn't solve the progress reporting problem.TaskCompletionSource<T>combined with running the native call on a background thread and marshaling progress reports properly is the correct approach.Current Pattern (example:
AddPackage)Key classes involved:
DismProgress(insrc/Microsoft.Dism/DismProgress.cs) — wraps the native callback, hasCancelproperty that signals anEventWaitHandle, hasCurrent/Total/UserDatapropertiesDismProgressCallback(insrc/Microsoft.Dism/DismProgressCallback.cs) — public delegate:public delegate void DismProgressCallback(DismProgress progress);internal delegate void DismProgressCallback(UInt32 current, UInt32 total, IntPtr userData);Requirements
1. Add
*Asyncmethods usingTaskCompletionSource<T>For each method that currently accepts a
DismProgressCallback, add an async counterpart usingTaskCompletionSource<T>. The async methods should:MethodNameAsyncnaming conventionIProgress<DismProgress>?instead ofDismProgressCallback?for progress reporting (standard .NET pattern)CancellationTokenfor cancellation (standard .NET pattern)Task(since the sync methods return void)TaskCompletionSource<bool>(or similar) to create the TaskTask.Factory.StartNewwithTaskCreationOptions.LongRunningorThread)IProgress<DismProgress>.Report()inside the progress callback to marshal progress updatesCancellationTokencancellation toDismProgress.Cancel = true(which signals the nativeEventWaitHandle)TaskCompletionSourceTaskCompletionSource.TrySetCanceled()TaskCompletionSource.TrySetException()2. Example of what an async method should look like