Skip to content

Commit c93494b

Browse files
Fix ordering of startup tasks so psEditor is defined before profiles are loaded (#1782)
The thing is, `RequiresForeground` necessarily implies an execution priority of `Next`. So instead we set that priority explicitly for `psEditor` and mark none of the startup tasks as requiring foreground, because they're all started before we enter the REPL where that matters.
1 parent 459d717 commit c93494b

File tree

5 files changed

+28
-13
lines changed

5 files changed

+28
-13
lines changed

src/PowerShellEditorServices/Services/Extension/ExtensionService.cs

+3-2
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,11 @@ internal Task InitializeAsync()
104104
// This is constant so Remove-Variable cannot remove it.
105105
PSVariable psEditor = new(PSEditorVariableName, EditorObject, ScopedItemOptions.Constant);
106106

107-
// Register the editor object in the runspace
107+
// NOTE: This is a special task run on startup! Register the editor object in the
108+
// runspace. It has priority next so it goes before LoadProfiles.
108109
return ExecutionService.ExecuteDelegateAsync(
109110
$"Create ${PSEditorVariableName} object",
110-
executionOptions: null,
111+
new ExecutionOptions { Priority = ExecutionPriority.Next },
111112
(pwsh, _) => pwsh.Runspace.SessionStateProxy.PSVariable.Set(psEditor),
112113
CancellationToken.None);
113114
}

src/PowerShellEditorServices/Services/PowerShell/Execution/ExecutionOptions.cs

+3
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@ public enum ExecutionPriority
1515
// Generally the executor will do the right thing though; some options just priority over others.
1616
public record ExecutionOptions
1717
{
18+
// This determines which underlying queue the task is added to.
1819
public ExecutionPriority Priority { get; init; } = ExecutionPriority.Normal;
20+
// This implies `ExecutionPriority.Next` because foreground tasks are prepended.
1921
public bool RequiresForeground { get; init; }
2022
}
2123

2224
public record PowerShellExecutionOptions : ExecutionOptions
2325
{
26+
// TODO: Because of the above, this is actually unnecessary.
2427
internal static PowerShellExecutionOptions ImmediateInteractive = new()
2528
{
2629
Priority = ExecutionPriority.Next,

src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs

+12-8
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) Microsoft Corporation.
1+
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

44
using System;
@@ -277,6 +277,8 @@ public void SetExit()
277277
public Task<T> InvokeTaskOnPipelineThreadAsync<T>(
278278
SynchronousTask<T> task)
279279
{
280+
// NOTE: This causes foreground tasks to act like they have `ExecutionPriority.Next`.
281+
// TODO: Deduplicate this.
280282
if (task.ExecutionOptions.RequiresForeground)
281283
{
282284
// When a task must displace the current foreground command,
@@ -296,6 +298,7 @@ public Task<T> InvokeTaskOnPipelineThreadAsync<T>(
296298
return task.Task;
297299
}
298300

301+
// TODO: Apply stashed `QueueTask` function.
299302
switch (task.ExecutionOptions.Priority)
300303
{
301304
case ExecutionPriority.Next:
@@ -404,14 +407,10 @@ public void InvokePSDelegate(string representation, ExecutionOptions executionOp
404407

405408
internal Task LoadHostProfilesAsync(CancellationToken cancellationToken)
406409
{
407-
// TODO: Why exactly does loading profiles require the foreground?
410+
// NOTE: This is a special task run on startup!
408411
return ExecuteDelegateAsync(
409412
"LoadProfiles",
410-
new PowerShellExecutionOptions
411-
{
412-
RequiresForeground = true,
413-
ThrowOnError = false
414-
},
413+
new PowerShellExecutionOptions { ThrowOnError = false },
415414
(pwsh, _) => pwsh.LoadProfiles(_hostInfo.ProfilePaths),
416415
cancellationToken);
417416
}
@@ -625,7 +624,10 @@ private void RunTopLevelExecutionLoop()
625624
{
626625
try
627626
{
628-
// Make sure we execute any startup tasks first
627+
// Make sure we execute any startup tasks first. These should be, in order:
628+
// 1. Delegate to register psEditor variable
629+
// 2. LoadProfiles delegate
630+
// 3. Delegate to import PSEditModule
629631
while (_taskQueue.TryTake(out ISynchronousTask task))
630632
{
631633
task.ExecuteSynchronously(CancellationToken.None);
@@ -1003,6 +1005,8 @@ private void OnPowerShellIdle(CancellationToken idleCancellationToken)
10031005
while (!cancellationScope.CancellationToken.IsCancellationRequested
10041006
&& _taskQueue.TryTake(out ISynchronousTask task))
10051007
{
1008+
// NOTE: This causes foreground tasks to act like they have `ExecutionPriority.Next`.
1009+
// TODO: Deduplicate this.
10061010
if (task.ExecutionOptions.RequiresForeground)
10071011
{
10081012
// If we have a task that is queued, but cannot be run under readline

src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs

+7-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ public override async Task<Unit> Handle(DidChangeConfigurationParams request, Ca
9797
}
9898
}
9999

100-
// TODO: Load profiles when the host is already running
100+
// TODO: Load profiles when the host is already running? Note that this might mess up
101+
// the ordering and require the foreground.
101102
if (!_cwdSet)
102103
{
103104
if (!string.IsNullOrEmpty(_configurationService.CurrentSettings.Cwd)
@@ -124,6 +125,11 @@ await _psesHost.SetInitialWorkingDirectoryAsync(
124125
_cwdSet = true;
125126
}
126127

128+
// This is another place we call this to setup $psEditor, which really needs to be done
129+
// _before_ profiles. In current testing, this has already been done by the call to
130+
// InitializeAsync when the ExtensionService class is injected.
131+
//
132+
// TODO: Remove this.
127133
await _extensionService.InitializeAsync().ConfigureAwait(false);
128134

129135
// Run any events subscribed to configuration updates

src/PowerShellEditorServices/Services/Workspace/RemoteFileManagerService.cs

+3-2
Original file line numberDiff line numberDiff line change
@@ -645,9 +645,10 @@ private async void HandlePSEventReceivedAsync(object sender, PSEventArgs args)
645645
}
646646
}
647647

648+
// NOTE: This is a special task run on startup!
648649
private Task RegisterPSEditFunctionAsync()
649650
=> _executionService.ExecuteDelegateAsync(
650-
"Register psedit function",
651+
"Register PSEdit function",
651652
executionOptions: null,
652653
(pwsh, _) => RegisterPSEditFunction(pwsh.Runspace),
653654
CancellationToken.None);
@@ -673,7 +674,7 @@ private void RegisterPSEditFunction(Runspace runspace)
673674
}
674675
catch (Exception e)
675676
{
676-
logger.LogException("Could not create psedit function.", e);
677+
logger.LogException("Could not create PSEdit function.", e);
677678
}
678679
finally
679680
{

0 commit comments

Comments
 (0)