Skip to content

Commit 1ff0b28

Browse files
msftrubengumsftrubengu
authored
Improve repair (#3886)
This PR makes some improvements to the `Microsoft.WinGet.Client` module **Use stub preference for AppInstaller** The default behavior for `Add-AppxPackage` is to install the full package regardless of the preference. This PR adds `-StubPreference UsePreference` (if the cmdlet supports it) when installing the AppInstaller bundle. I originally want to allow the user to set the preference, but I realized I would have to blindly call `winget configure --enable|disable` since I don't know the state. I believe this should be done differently with perhaps different cmdlets and add something like `winget config --info` that prints a json with some information like if its enabled or not and the processor info. **Add -Force to Repair-WinGetPackage** On some occasions, `Add-AppxPackage` failed because AppInstaller files were in use. I added a `-Force` switch to the cmdlet that just sets `-ForceTargetApplicationShutdown` on the add appx package call. **Module async friendly** All async functions in the module required to be waited. Now that we can start an async context via `RunOnMTA`, I removed all the synchronous wrappers calls from GitHubClient and make it such as only the top-level cmdlet need to wait on the Task. This caused a lot of functions to be awaited, but at the end only the top-level cmdlet should care about manually waiting for the task which I believe is the right thing to do. **Bring back synchronization with main PowerShell thread** AppxModuleHelper requires its Appx Module calls to be executed in the main PowerShell thread otherwise a new runtime would need to be created for every cmdlet that uses this class. I brought back the old synchronization mechanism of `Microsoft.WinGet.Configure` to do this. Now any cmdlet that runs asynchronously and requires PowerShell Host can do it without creating its runspace. **Redo progress for Install/Update/Uninstall-WinGetPackage** I was not a huge fan of how progress was handled in these cmdlets. Given that I can now create an asynchronous context, I simplify how progress is done and follow a similar pattern as `Microsoft.WinGet.Configure`. The `IAsyncOperationWithProgress` is executed, set its Progress handler, converted to a Task and awaited all in one common method. I also fixed how the progress was calculated so it reflects almost the same as winget. .NET rounded the number so for some package I was 92.3 MB in winget and 92.4 the module (before it was like 97or something). **Add proper can cancellation support** Modify the Install/Update/Uninstall cmdlets to properly use PowerShell's `StopProcessing` for cancellation. The Task from `IAsyncOperationWithProgress` uses the PowerShellCmdlet CancellationToken, so everything is handled correctly. Also add cancellation to `Repair-WinGetPackage`. The repair state machine sees if the token is cancelled as the first thing on the loop. **Download URL with progress.** The module attempts to install a package using `Add-AppxPackage` with the url of the package. In the case it fails, like in Windows Sandbox, the package is downloaded to disk and then installed via the same cmdlet. Depending on the network, the download could take a while and there's no indication that work is being performed without `-Verbose`. This PR adds a PowerShell progress bar for the download when the response contains a Content-Length property. If cancellation is request during url download, the async calls will be cancelled the file will be deleted.
1 parent fb540a2 commit 1ff0b28

27 files changed

Lines changed: 901 additions & 485 deletions

‎src/PowerShell/CommonFiles/PowerShellCmdlet.cs‎

Lines changed: 137 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace Microsoft.WinGet.Common.Command
1010
using System.Collections.Concurrent;
1111
using System.Collections.Generic;
1212
using System.Management.Automation;
13+
using System.Runtime.ExceptionServices;
1314
using System.Threading;
1415
using System.Threading.Tasks;
1516
using Microsoft.WinGet.Resources;
@@ -30,13 +31,18 @@ public abstract class PowerShellCmdlet
3031
private static readonly string[] WriteInformationTags = new string[] { "PSHOST" };
3132

3233
private readonly PSCmdlet psCmdlet;
33-
private readonly Thread originalThread;
34+
private readonly Thread pwshThread;
3435

3536
private readonly CancellationTokenSource source = new ();
36-
private BlockingCollection<QueuedStream> queuedStreams = new ();
37+
private readonly SemaphoreSlim semaphore = new (1, 1);
38+
private readonly ManualResetEventSlim pwshThreadActionReady = new (false);
39+
private readonly ManualResetEventSlim pwshThreadActionCompleted = new (false);
3740

41+
private BlockingCollection<QueuedStream> queuedStreams = new ();
3842
private int progressActivityId = 0;
3943
private ConcurrentDictionary<int, ProgressRecordType> progressRecords = new ();
44+
private Action? pwshThreadAction = null;
45+
private ExceptionDispatchInfo? pwshThreadEdi = null;
4046

4147
/// <summary>
4248
/// Initializes a new instance of the <see cref="PowerShellCmdlet"/> class.
@@ -57,7 +63,7 @@ public PowerShellCmdlet(PSCmdlet psCmdlet, HashSet<Policy> policies)
5763
this.ValidatePolicies(policies);
5864

5965
this.psCmdlet = psCmdlet;
60-
this.originalThread = Thread.CurrentThread;
66+
this.pwshThread = Thread.CurrentThread;
6167
}
6268

6369
/// <summary>
@@ -81,7 +87,7 @@ internal Task RunOnMTA(Func<Task> func)
8187
throw new NotImplementedException();
8288
#else
8389
// This must be called in the main thread.
84-
if (this.originalThread != Thread.CurrentThread)
90+
if (this.pwshThread != Thread.CurrentThread)
8591
{
8692
throw new InvalidOperationException();
8793
}
@@ -134,7 +140,7 @@ internal Task RunOnMTA(Func<Task> func)
134140
internal Task<TResult> RunOnMTA<TResult>(Func<Task<TResult>> func)
135141
{
136142
// This must be called in the main thread.
137-
if (this.originalThread != Thread.CurrentThread)
143+
if (this.pwshThread != Thread.CurrentThread)
138144
{
139145
throw new InvalidOperationException();
140146
}
@@ -186,7 +192,7 @@ internal Task<TResult> RunOnMTA<TResult>(Func<Task<TResult>> func)
186192
internal TResult RunOnMTA<TResult>(Func<TResult> func)
187193
{
188194
// This must be called in the main thread.
189-
if (this.originalThread != Thread.CurrentThread)
195+
if (this.pwshThread != Thread.CurrentThread)
190196
{
191197
throw new InvalidOperationException();
192198
}
@@ -229,6 +235,26 @@ internal TResult RunOnMTA<TResult>(Func<TResult> func)
229235
return tcs.Task.Result;
230236
}
231237

238+
/// <summary>
239+
/// Executes an action in the main thread.
240+
/// Blocks until call is executed.
241+
/// </summary>
242+
/// <param name="action">Action to perform.</param>
243+
internal void ExecuteInPowerShellThread(Action action)
244+
{
245+
if (this.pwshThread == Thread.CurrentThread)
246+
{
247+
action();
248+
return;
249+
}
250+
251+
this.WaitForOurTurn();
252+
253+
this.pwshThreadAction = action;
254+
this.pwshThreadActionReady.Set();
255+
this.WaitMainThreadActionCompletion();
256+
}
257+
232258
/// <summary>
233259
/// Waits for the task to be completed. This MUST be called from the main thread.
234260
/// </summary>
@@ -239,21 +265,67 @@ internal void Wait(Task runningTask, PowerShellCmdlet? writeCmdlet = null)
239265
writeCmdlet ??= this;
240266

241267
// This must be called in the main thread.
242-
if (this.originalThread != Thread.CurrentThread)
268+
if (this.pwshThread != Thread.CurrentThread)
243269
{
244270
throw new InvalidOperationException();
245271
}
246272

247273
do
248274
{
249-
this.ConsumeAndWriteStreams(writeCmdlet);
275+
if (this.pwshThreadActionReady.IsSet)
276+
{
277+
// Someone needs the main thread.
278+
this.pwshThreadActionReady.Reset();
279+
280+
if (this.pwshThreadAction != null)
281+
{
282+
try
283+
{
284+
this.pwshThreadAction();
285+
}
286+
catch (Exception e)
287+
{
288+
// Make sure we don't throw in the PowerShell thread, this way
289+
// we'll get a more meaningful stack by Get-Error.
290+
this.pwshThreadEdi = ExceptionDispatchInfo.Capture(e);
291+
}
292+
293+
this.pwshThreadAction = null;
294+
}
295+
296+
// Done.
297+
this.pwshThreadActionCompleted.Set();
298+
}
299+
300+
// Take from the blocking collection.
301+
if (!this.queuedStreams.IsCompleted && this.queuedStreams.Count > 0)
302+
{
303+
try
304+
{
305+
var queuedOutput = this.queuedStreams.Take();
306+
if (queuedOutput != null)
307+
{
308+
this.CmdletWrite(queuedOutput.Type, queuedOutput.Data, writeCmdlet);
309+
}
310+
}
311+
catch (InvalidOperationException)
312+
{
313+
// An InvalidOperationException means that Take() was called on a completed collection.
314+
}
315+
}
250316
}
251317
while (!(runningTask.IsCompleted && this.queuedStreams.IsCompleted));
252318

253319
if (runningTask.IsFaulted)
254320
{
255321
// If IsFaulted is true, the task's Status is equal to Faulted,
256322
// and its Exception property will be non-null.
323+
AggregateException? ae = runningTask.Exception! as AggregateException;
324+
if (ae != null && ae.InnerExceptions.Count == 1)
325+
{
326+
ExceptionDispatchInfo.Capture(ae.InnerExceptions[0]).Throw();
327+
}
328+
257329
throw runningTask.Exception!;
258330
}
259331
}
@@ -269,15 +341,17 @@ internal void Write(StreamType type, object data)
269341
{
270342
if (type == StreamType.Progress)
271343
{
272-
// Keep track of all progress activity.
273344
ProgressRecord progressRecord = (ProgressRecord)data;
274-
if (!this.progressRecords.TryAdd(progressRecord.ActivityId, progressRecord.RecordType))
345+
if (progressRecord.RecordType == ProgressRecordType.Completed)
275346
{
276-
_ = this.progressRecords.TryUpdate(progressRecord.ActivityId, progressRecord.RecordType, ProgressRecordType.Completed);
347+
throw new NotSupportedException("Use CompleteProgress");
277348
}
349+
350+
// Keep track of all progress activity.
351+
_ = this.progressRecords.TryAdd(progressRecord.ActivityId, progressRecord.RecordType);
278352
}
279353

280-
if (this.originalThread == Thread.CurrentThread)
354+
if (this.pwshThread == Thread.CurrentThread)
281355
{
282356
this.CmdletWrite(type, data, this);
283357
return;
@@ -311,26 +385,49 @@ internal void WriteProgressWithPercentage(int activityId, string activity, strin
311385
/// <param name="activityId">Activity id.</param>
312386
/// <param name="activity">The activity in progress.</param>
313387
/// <param name="status">The status of the activity.</param>
314-
internal void CompleteProgress(int activityId, string activity, string status)
388+
/// <param name="force">Force write complete progress.</param>
389+
internal void CompleteProgress(int activityId, string activity, string status, bool force = false)
315390
{
316391
var record = new ProgressRecord(activityId, activity, status)
317392
{
318393
RecordType = ProgressRecordType.Completed,
319394
PercentComplete = 100,
320395
};
321-
this.Write(StreamType.Progress, record);
396+
397+
if (!this.progressRecords.TryAdd(activityId, record.RecordType))
398+
{
399+
_ = this.progressRecords.TryUpdate(activityId, record.RecordType, ProgressRecordType.Processing);
400+
}
401+
402+
if (this.pwshThread == Thread.CurrentThread)
403+
{
404+
this.CmdletWrite(StreamType.Progress, record, this);
405+
}
406+
else
407+
{
408+
// You should only use force if you know the cmdlet that is completing this progress is a sync cmdlet that
409+
// is running in an async context. A sync cmdlet is anything that doesn't start with Start-*
410+
if (force)
411+
{
412+
this.ExecuteInPowerShellThread(() => this.CmdletWrite(StreamType.Progress, record, this));
413+
}
414+
else
415+
{
416+
this.queuedStreams.Add(new QueuedStream(StreamType.Progress, record));
417+
}
418+
}
322419
}
323420

324421
/// <summary>
325422
/// Writes to PowerShell streams.
326423
/// This method must be called in the original thread.
327-
/// WARNING: You must only call this when the task is completed or in Wait.
424+
/// WARNING: You must only call this when the task is completed.
328425
/// </summary>
329426
/// <param name="writeCmdlet">The cmdlet that can write to PowerShell.</param>
330427
internal void ConsumeAndWriteStreams(PowerShellCmdlet writeCmdlet)
331428
{
332429
// This must be called in the main thread.
333-
if (this.originalThread != Thread.CurrentThread)
430+
if (this.pwshThread != Thread.CurrentThread)
334431
{
335432
throw new InvalidOperationException();
336433
}
@@ -399,7 +496,7 @@ internal void SetVariable(string variableName, object value)
399496
internal bool ShouldProcess(string target)
400497
{
401498
// If not on the main thread just continue.
402-
if (this.originalThread != Thread.CurrentThread)
499+
if (this.pwshThread != Thread.CurrentThread)
403500
{
404501
return true;
405502
}
@@ -430,7 +527,7 @@ private void CmdletWrite(StreamType streamType, object data, PowerShellCmdlet wr
430527
case StreamType.Progress:
431528
// If the activity is already completed don't write progress.
432529
var progressRecord = (ProgressRecord)data;
433-
if (this.progressRecords[progressRecord.ActivityId] == ProgressRecordType.Processing)
530+
if (this.progressRecords[progressRecord.ActivityId] == progressRecord.RecordType)
434531
{
435532
writeCmdlet.psCmdlet.WriteProgress(progressRecord);
436533
}
@@ -485,6 +582,28 @@ private void ValidatePolicies(HashSet<Policy> policies)
485582
}
486583
}
487584

585+
private void WaitForOurTurn()
586+
{
587+
this.semaphore.Wait(this.GetCancellationToken());
588+
this.pwshThreadActionCompleted.Reset();
589+
}
590+
591+
private void WaitMainThreadActionCompletion()
592+
{
593+
WaitHandle.WaitAny(new[]
594+
{
595+
this.GetCancellationToken().WaitHandle,
596+
this.pwshThreadActionCompleted.WaitHandle,
597+
});
598+
599+
if (this.pwshThreadEdi != null)
600+
{
601+
this.pwshThreadEdi.Throw();
602+
}
603+
604+
this.semaphore.Release();
605+
}
606+
488607
private class QueuedStream
489608
{
490609
public QueuedStream(StreamType type, object data)

‎src/PowerShell/Microsoft.WinGet.Client.Cmdlets/Cmdlets/InstallPackageCmdlet.cs‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ namespace Microsoft.WinGet.Client.Commands
2424
[OutputType(typeof(PSInstallResult))]
2525
public sealed class InstallPackageCmdlet : InstallCmdlet
2626
{
27+
private InstallerPackageCommand command = null;
28+
2729
/// <summary>
2830
/// Gets or sets the scope to install the application under.
2931
/// </summary>
@@ -41,9 +43,8 @@ public sealed class InstallPackageCmdlet : InstallCmdlet
4143
/// </summary>
4244
protected override void ProcessRecord()
4345
{
44-
var command = new InstallerPackageCommand(
46+
this.command = new InstallerPackageCommand(
4547
this,
46-
this.Mode.ToString(),
4748
this.Override,
4849
this.Custom,
4950
this.Location,
@@ -58,7 +59,18 @@ protected override void ProcessRecord()
5859
this.Moniker,
5960
this.Source,
6061
this.Query);
61-
command.Install(this.Scope.ToString(), this.Architecture.ToString(), this.MatchOption.ToString(), this.Mode.ToString());
62+
this.command.Install(this.Scope.ToString(), this.Architecture.ToString(), this.MatchOption.ToString(), this.Mode.ToString());
63+
}
64+
65+
/// <summary>
66+
/// Interrupts currently running code within the command.
67+
/// </summary>
68+
protected override void StopProcessing()
69+
{
70+
if (this.command != null)
71+
{
72+
this.command.Cancel();
73+
}
6274
}
6375
}
6476
}

‎src/PowerShell/Microsoft.WinGet.Client.Cmdlets/Cmdlets/RepairWinGetPackageManagerCmdlet.cs‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,26 +21,45 @@ namespace Microsoft.WinGet.Client.Commands
2121
[OutputType(typeof(int))]
2222
public class RepairWinGetPackageManagerCmdlet : WinGetPackageManagerCmdlet
2323
{
24+
private WinGetPackageManagerCommand command = null;
25+
2426
/// <summary>
2527
/// Gets or sets a value indicating whether to repair for all users. Requires admin.
2628
/// </summary>
2729
[Parameter(ValueFromPipelineByPropertyName = true)]
2830
public SwitchParameter AllUsers { get; set; }
2931

32+
/// <summary>
33+
/// Gets or sets a value indicating whether to force application shutdown.
34+
/// </summary>
35+
[Parameter(ValueFromPipelineByPropertyName = true)]
36+
public SwitchParameter Force { get; set; }
37+
3038
/// <summary>
3139
/// Attempts to repair winget.
3240
/// TODO: consider WhatIf and Confirm options.
3341
/// </summary>
3442
protected override void ProcessRecord()
3543
{
36-
var command = new WinGetPackageManagerCommand(this);
44+
this.command = new WinGetPackageManagerCommand(this);
3745
if (this.ParameterSetName == Constants.IntegrityLatestSet)
3846
{
39-
command.RepairUsingLatest(this.IncludePreRelease.ToBool(), this.AllUsers.ToBool());
47+
this.command.RepairUsingLatest(this.IncludePreRelease.ToBool(), this.AllUsers.ToBool(), this.Force.ToBool());
4048
}
4149
else
4250
{
43-
command.Repair(this.Version, this.AllUsers.ToBool());
51+
this.command.Repair(this.Version, this.AllUsers.ToBool(), this.Force.ToBool());
52+
}
53+
}
54+
55+
/// <summary>
56+
/// Interrupts currently running code within the command.
57+
/// </summary>
58+
protected override void StopProcessing()
59+
{
60+
if (this.command != null)
61+
{
62+
this.command.Cancel();
4463
}
4564
}
4665
}

0 commit comments

Comments
 (0)