From ba1aa62209a367d58555cb86b773c825f50a0eae Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Mon, 24 Feb 2020 11:49:26 -0800 Subject: [PATCH 1/7] Improve startup task dispose --- .../Commands/StartEditorServicesCommand.cs | 3 ++- .../Configuration/HostLogger.cs | 2 +- .../EditorServicesLoader.cs | 23 +++++++++++-------- .../Internal/EditorServicesRunner.cs | 4 ++-- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs index e27de4c42..7d590d60c 100644 --- a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs +++ b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs @@ -25,6 +25,7 @@ using System.Threading; using Debugger = System.Diagnostics.Debugger; +using System.Threading.Tasks; #endif namespace Microsoft.PowerShell.EditorServices.Commands @@ -238,7 +239,7 @@ protected override void EndProcessing() using (var psesLoader = EditorServicesLoader.Create(_logger, editorServicesConfig, sessionFileWriter, _loggerUnsubscribers)) { _logger.Log(PsesLogLevel.Verbose, "Loading EditorServices"); - psesLoader.LoadAndRunEditorServicesAsync().Wait(); + psesLoader.LoadAndRunEditorServicesAsync().GetAwaiter().GetResult(); } } catch (Exception e) diff --git a/src/PowerShellEditorServices.Hosting/Configuration/HostLogger.cs b/src/PowerShellEditorServices.Hosting/Configuration/HostLogger.cs index 704dde9ab..af3404aaa 100644 --- a/src/PowerShellEditorServices.Hosting/Configuration/HostLogger.cs +++ b/src/PowerShellEditorServices.Hosting/Configuration/HostLogger.cs @@ -363,7 +363,7 @@ private void RunWriter() _fileWriter.WriteLine(logMessage); } } - catch (TaskCanceledException) + catch (OperationCanceledException) { } } diff --git a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs index dc35d9c3b..3f2d7b407 100644 --- a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs +++ b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs @@ -160,6 +160,8 @@ public static EditorServicesLoader Create( private readonly IReadOnlyCollection _loggersToUnsubscribe; + private EditorServicesRunner _editorServicesRunner; + private EditorServicesLoader( HostLogger logger, EditorServicesConfig hostConfig, @@ -177,7 +179,7 @@ private EditorServicesLoader( /// This method's returned task will end when Editor Services shuts down. /// /// - public async Task LoadAndRunEditorServicesAsync() + public Task LoadAndRunEditorServicesAsync() { // Log important host information here LogHostInformation(); @@ -200,18 +202,21 @@ public async Task LoadAndRunEditorServicesAsync() LoadEditorServices(); _logger.Log(PsesLogLevel.Verbose, "Starting EditorServices"); - using (var editorServicesRunner = new EditorServicesRunner(_logger, _hostConfig, _sessionFileWriter, _loggersToUnsubscribe)) - { - // The trigger method for Editor Services - // We will wait here until Editor Services shuts down - await editorServicesRunner.RunUntilShutdown().ConfigureAwait(false); - } + + _editorServicesRunner = new EditorServicesRunner(_logger, _hostConfig, _sessionFileWriter, _loggersToUnsubscribe); + + // The trigger method for Editor Services + // We will wait here until Editor Services shuts down + return Task.Run(_editorServicesRunner.RunUntilShutdown); } public void Dispose() { - // TODO: Remove assembly resolve events - // This is not high priority, since the PSES process shouldn't be reused + _editorServicesRunner.Dispose(); + + // TODO: + // Remove assembly resolve events + // This is not high priority, since the PSES process shouldn't be reused } private void LoadEditorServices() diff --git a/src/PowerShellEditorServices.Hosting/Internal/EditorServicesRunner.cs b/src/PowerShellEditorServices.Hosting/Internal/EditorServicesRunner.cs index dba26d2e7..cefc251ae 100644 --- a/src/PowerShellEditorServices.Hosting/Internal/EditorServicesRunner.cs +++ b/src/PowerShellEditorServices.Hosting/Internal/EditorServicesRunner.cs @@ -47,7 +47,7 @@ public EditorServicesRunner( /// Start and run Editor Services and then wait for shutdown. /// /// A task that ends when Editor Services shuts down. - public async Task RunUntilShutdown() + public Task RunUntilShutdown() { // Start Editor Services Task runAndAwaitShutdown = CreateEditorServicesAndRunUntilShutdown(); @@ -57,7 +57,7 @@ public async Task RunUntilShutdown() _sessionFileWriter.WriteSessionStarted(_config.LanguageServiceTransport, _config.DebugServiceTransport); // Finally, wait for Editor Services to shut down - await runAndAwaitShutdown.ConfigureAwait(false); + return runAndAwaitShutdown; } public void Dispose() From 24177c9de90c8401cea2a94eba3c2af2bf403d36 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Mon, 24 Feb 2020 11:53:47 -0800 Subject: [PATCH 2/7] Remove unused usings --- .../Commands/StartEditorServicesCommand.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs index 7d590d60c..6057282fe 100644 --- a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs +++ b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs @@ -21,11 +21,8 @@ #endif #if DEBUG -using System.Diagnostics; -using System.Threading; - using Debugger = System.Diagnostics.Debugger; -using System.Threading.Tasks; +using System.Threading; #endif namespace Microsoft.PowerShell.EditorServices.Commands From a1af6e11bd17e51d9f32fb53d2e60fa845950d87 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Mon, 24 Feb 2020 11:56:15 -0800 Subject: [PATCH 3/7] Make comment better --- .../Commands/StartEditorServicesCommand.cs | 2 ++ src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs index 6057282fe..0ec6d8243 100644 --- a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs +++ b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs @@ -22,6 +22,7 @@ #if DEBUG using Debugger = System.Diagnostics.Debugger; +using System.Diagnostics; using System.Threading; #endif @@ -236,6 +237,7 @@ protected override void EndProcessing() using (var psesLoader = EditorServicesLoader.Create(_logger, editorServicesConfig, sessionFileWriter, _loggerUnsubscribers)) { _logger.Log(PsesLogLevel.Verbose, "Loading EditorServices"); + // Start editor services and wait here until it shuts down psesLoader.LoadAndRunEditorServicesAsync().GetAwaiter().GetResult(); } } diff --git a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs index 3f2d7b407..5439159dc 100644 --- a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs +++ b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs @@ -206,7 +206,6 @@ public Task LoadAndRunEditorServicesAsync() _editorServicesRunner = new EditorServicesRunner(_logger, _hostConfig, _sessionFileWriter, _loggersToUnsubscribe); // The trigger method for Editor Services - // We will wait here until Editor Services shuts down return Task.Run(_editorServicesRunner.RunUntilShutdown); } From f255b880da468ea35847518ffc47b1b3fdd97e70 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Mon, 24 Feb 2020 11:58:08 -0800 Subject: [PATCH 4/7] Fix debugger using --- .../Commands/StartEditorServicesCommand.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs index 0ec6d8243..82a6a0eb5 100644 --- a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs +++ b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs @@ -21,9 +21,10 @@ #endif #if DEBUG -using Debugger = System.Diagnostics.Debugger; using System.Diagnostics; using System.Threading; + +using Debugger = System.Diagnostics.Debugger; #endif namespace Microsoft.PowerShell.EditorServices.Commands From 194101046fc9be9f6c580cc8ca86753bf96062b8 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Mon, 24 Feb 2020 11:59:36 -0800 Subject: [PATCH 5/7] Ensure no NRE --- src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs index 5439159dc..7c204de2e 100644 --- a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs +++ b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs @@ -211,7 +211,7 @@ public Task LoadAndRunEditorServicesAsync() public void Dispose() { - _editorServicesRunner.Dispose(); + _editorServicesRunner?.Dispose(); // TODO: // Remove assembly resolve events From 7d09ed5c7b28b022ac779de698133b28f8edbf2b Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Mon, 24 Feb 2020 12:04:57 -0800 Subject: [PATCH 6/7] Add documentation comment --- src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs index 7c204de2e..78e97f7f1 100644 --- a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs +++ b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs @@ -220,6 +220,8 @@ public void Dispose() private void LoadEditorServices() { + // This must be in its own method, since the actual load happens when the calling method is called + // The call within this method is therefore a total no-op EditorServicesLoading.LoadEditorServicesForHost(); } From be13d28ee4bf45ad76bb6c31675814ce3d3247ca Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Mon, 24 Feb 2020 15:14:09 -0800 Subject: [PATCH 7/7] Add more logging --- .../Commands/StartEditorServicesCommand.cs | 16 +++---- .../Configuration/TransportConfig.cs | 48 +++++++++++++------ .../EditorServicesLoader.cs | 1 + .../Internal/EditorServicesRunner.cs | 3 ++ 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs index 82a6a0eb5..9f7a88551 100644 --- a/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs +++ b/src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs @@ -433,20 +433,20 @@ private ITransportConfig GetLanguageServiceTransport() if (Stdio) { - return new StdioTransportConfig(); + return new StdioTransportConfig(_logger); } if (LanguageServiceInPipeName != null && LanguageServiceOutPipeName != null) { - return SimplexNamedPipeTransportConfig.Create(LanguageServiceInPipeName, LanguageServiceOutPipeName); + return SimplexNamedPipeTransportConfig.Create(_logger, LanguageServiceInPipeName, LanguageServiceOutPipeName); } if (SplitInOutPipes) { - return SimplexNamedPipeTransportConfig.Create(LanguageServicePipeName); + return SimplexNamedPipeTransportConfig.Create(_logger, LanguageServicePipeName); } - return DuplexNamedPipeTransportConfig.Create(LanguageServicePipeName); + return DuplexNamedPipeTransportConfig.Create(_logger, LanguageServicePipeName); } private ITransportConfig GetDebugServiceTransport() @@ -457,7 +457,7 @@ private ITransportConfig GetDebugServiceTransport() { if (DebugServiceOnly) { - return new StdioTransportConfig(); + return new StdioTransportConfig(_logger); } _logger.Log(PsesLogLevel.Diagnostic, "No debug transport: Transport is Stdio with debug disabled"); @@ -466,15 +466,15 @@ private ITransportConfig GetDebugServiceTransport() if (DebugServiceInPipeName != null && DebugServiceOutPipeName != null) { - return SimplexNamedPipeTransportConfig.Create(DebugServiceInPipeName, DebugServiceOutPipeName); + return SimplexNamedPipeTransportConfig.Create(_logger, DebugServiceInPipeName, DebugServiceOutPipeName); } if (SplitInOutPipes) { - return SimplexNamedPipeTransportConfig.Create(DebugServicePipeName); + return SimplexNamedPipeTransportConfig.Create(_logger, DebugServicePipeName); } - return DuplexNamedPipeTransportConfig.Create(DebugServicePipeName); + return DuplexNamedPipeTransportConfig.Create(_logger, DebugServicePipeName); } private enum ProfileHostKind diff --git a/src/PowerShellEditorServices.Hosting/Configuration/TransportConfig.cs b/src/PowerShellEditorServices.Hosting/Configuration/TransportConfig.cs index 2287169df..de6e73a5f 100644 --- a/src/PowerShellEditorServices.Hosting/Configuration/TransportConfig.cs +++ b/src/PowerShellEditorServices.Hosting/Configuration/TransportConfig.cs @@ -43,6 +43,13 @@ public interface ITransportConfig /// public sealed class StdioTransportConfig : ITransportConfig { + private readonly HostLogger _logger; + + public StdioTransportConfig(HostLogger logger) + { + _logger = logger; + } + public string EndpointDetails => ""; public string SessionFileTransportName => "Stdio"; @@ -51,6 +58,7 @@ public sealed class StdioTransportConfig : ITransportConfig public Task<(Stream inStream, Stream outStream)> ConnectStreamsAsync() { + _logger.Log(PsesLogLevel.Diagnostic, "Connecting stdio streams"); return Task.FromResult((Console.OpenStandardInput(), Console.OpenStandardOutput())); } } @@ -64,29 +72,32 @@ public sealed class DuplexNamedPipeTransportConfig : ITransportConfig /// Create a duplex named pipe transport config with an automatically generated pipe name. /// /// A new duplex named pipe transport configuration. - public static DuplexNamedPipeTransportConfig Create() + public static DuplexNamedPipeTransportConfig Create(HostLogger logger) { - return new DuplexNamedPipeTransportConfig(NamedPipeUtils.GenerateValidNamedPipeName()); + return new DuplexNamedPipeTransportConfig(logger, NamedPipeUtils.GenerateValidNamedPipeName()); } /// /// Create a duplex named pipe transport config with the given pipe name. /// /// A new duplex named pipe transport configuration. - public static DuplexNamedPipeTransportConfig Create(string pipeName) + public static DuplexNamedPipeTransportConfig Create(HostLogger logger, string pipeName) { if (pipeName == null) { - return DuplexNamedPipeTransportConfig.Create(); + return DuplexNamedPipeTransportConfig.Create(logger); } - return new DuplexNamedPipeTransportConfig(pipeName); + return new DuplexNamedPipeTransportConfig(logger, pipeName); } + private readonly HostLogger _logger; + private readonly string _pipeName; - private DuplexNamedPipeTransportConfig(string pipeName) + private DuplexNamedPipeTransportConfig(HostLogger logger, string pipeName) { + _logger = logger; _pipeName = pipeName; SessionFileEntries = new Dictionary{ { "PipeName", NamedPipeUtils.GetNamedPipePath(pipeName) } }; } @@ -99,8 +110,11 @@ private DuplexNamedPipeTransportConfig(string pipeName) public async Task<(Stream inStream, Stream outStream)> ConnectStreamsAsync() { + _logger.Log(PsesLogLevel.Diagnostic, "Creating named pipe"); NamedPipeServerStream namedPipe = NamedPipeUtils.CreateNamedPipe(_pipeName, PipeDirection.InOut); + _logger.Log(PsesLogLevel.Diagnostic, "Waiting for named pipe connection"); await namedPipe.WaitForConnectionAsync().ConfigureAwait(false); + _logger.Log(PsesLogLevel.Diagnostic, "Named pipe connected"); return (namedPipe, namedPipe); } } @@ -117,42 +131,44 @@ public sealed class SimplexNamedPipeTransportConfig : ITransportConfig /// Create a pair of simplex named pipes using generated names. /// /// A new simplex named pipe transport config. - public static SimplexNamedPipeTransportConfig Create() + public static SimplexNamedPipeTransportConfig Create(HostLogger logger) { - return SimplexNamedPipeTransportConfig.Create(NamedPipeUtils.GenerateValidNamedPipeName(new[] { InPipePrefix, OutPipePrefix })); + return SimplexNamedPipeTransportConfig.Create(logger, NamedPipeUtils.GenerateValidNamedPipeName(new[] { InPipePrefix, OutPipePrefix })); } /// /// Create a pair of simplex named pipes using the given name as a base. /// /// A new simplex named pipe transport config. - public static SimplexNamedPipeTransportConfig Create(string pipeNameBase) + public static SimplexNamedPipeTransportConfig Create(HostLogger logger, string pipeNameBase) { if (pipeNameBase == null) { - return SimplexNamedPipeTransportConfig.Create(); + return SimplexNamedPipeTransportConfig.Create(logger); } string inPipeName = $"{InPipePrefix}_{pipeNameBase}"; string outPipeName = $"{OutPipePrefix}_{pipeNameBase}"; - return SimplexNamedPipeTransportConfig.Create(inPipeName, outPipeName); + return SimplexNamedPipeTransportConfig.Create(logger, inPipeName, outPipeName); } /// /// Create a pair of simplex named pipes using the given names. /// /// A new simplex named pipe transport config. - public static SimplexNamedPipeTransportConfig Create(string inPipeName, string outPipeName) + public static SimplexNamedPipeTransportConfig Create(HostLogger logger, string inPipeName, string outPipeName) { - return new SimplexNamedPipeTransportConfig(inPipeName, outPipeName); + return new SimplexNamedPipeTransportConfig(logger, inPipeName, outPipeName); } + private readonly HostLogger _logger; private readonly string _inPipeName; private readonly string _outPipeName; - private SimplexNamedPipeTransportConfig(string inPipeName, string outPipeName) + private SimplexNamedPipeTransportConfig(HostLogger logger, string inPipeName, string outPipeName) { + _logger = logger; _inPipeName = inPipeName; _outPipeName = outPipeName; @@ -171,14 +187,18 @@ private SimplexNamedPipeTransportConfig(string inPipeName, string outPipeName) public async Task<(Stream inStream, Stream outStream)> ConnectStreamsAsync() { + _logger.Log(PsesLogLevel.Diagnostic, "Starting in pipe connection"); NamedPipeServerStream inPipe = NamedPipeUtils.CreateNamedPipe(_inPipeName, PipeDirection.InOut); Task inPipeConnected = inPipe.WaitForConnectionAsync(); + _logger.Log(PsesLogLevel.Diagnostic, "Starting out pipe connection"); NamedPipeServerStream outPipe = NamedPipeUtils.CreateNamedPipe(_outPipeName, PipeDirection.Out); Task outPipeConnected = outPipe.WaitForConnectionAsync(); + _logger.Log(PsesLogLevel.Diagnostic, "Wating for pipe connections"); await Task.WhenAll(inPipeConnected, outPipeConnected).ConfigureAwait(false); + _logger.Log(PsesLogLevel.Diagnostic, "Simplex named pipe transport connected"); return (inPipe, outPipe); } } diff --git a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs index 78e97f7f1..7c69a895d 100644 --- a/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs +++ b/src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs @@ -211,6 +211,7 @@ public Task LoadAndRunEditorServicesAsync() public void Dispose() { + _logger.Log(PsesLogLevel.Diagnostic, "Loader disposed"); _editorServicesRunner?.Dispose(); // TODO: diff --git a/src/PowerShellEditorServices.Hosting/Internal/EditorServicesRunner.cs b/src/PowerShellEditorServices.Hosting/Internal/EditorServicesRunner.cs index cefc251ae..cd8ae8013 100644 --- a/src/PowerShellEditorServices.Hosting/Internal/EditorServicesRunner.cs +++ b/src/PowerShellEditorServices.Hosting/Internal/EditorServicesRunner.cs @@ -57,6 +57,7 @@ public Task RunUntilShutdown() _sessionFileWriter.WriteSessionStarted(_config.LanguageServiceTransport, _config.DebugServiceTransport); // Finally, wait for Editor Services to shut down + _logger.Log(PsesLogLevel.Diagnostic, "Waiting on PSES run/shutdown"); return runAndAwaitShutdown; } @@ -73,6 +74,8 @@ private async Task CreateEditorServicesAndRunUntilShutdown() { try { + _logger.Log(PsesLogLevel.Diagnostic, "Creating/running editor services"); + bool creatingLanguageServer = _config.LanguageServiceTransport != null; bool creatingDebugServer = _config.DebugServiceTransport != null; bool isTempDebugSession = creatingDebugServer && !creatingLanguageServer;