From f55aec42d1033d2ffbe9e8d876e8d3c3de2cff70 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Mon, 29 Apr 2019 14:20:14 -0700 Subject: [PATCH 1/8] Fix no runspace available --- src/PowerShell/PowerShellManager.cs | 25 ++++++++++++++++++- src/PowerShell/PowerShellManagerPool.cs | 20 +++++++++++++++ src/RequestProcessor.cs | 4 +++ src/resources/PowerShellWorkerStrings.resx | 3 +++ .../Unit/PowerShell/PowerShellManagerTests.cs | 15 +++++++++-- 5 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/PowerShell/PowerShellManager.cs b/src/PowerShell/PowerShellManager.cs index d753ffa2..147afb90 100644 --- a/src/PowerShell/PowerShellManager.cs +++ b/src/PowerShell/PowerShellManager.cs @@ -22,6 +22,7 @@ internal class PowerShellManager { private readonly ILogger _logger; private readonly PowerShell _pwsh; + private bool shouldInvokeProfile; /// /// Gets the Runspace InstanceId. @@ -45,7 +46,7 @@ static PowerShellManager() addMethod.Invoke(null, new object[] { "HttpRequestContext", typeof(HttpRequestContext) }); } - internal PowerShellManager(ILogger logger) + internal PowerShellManager(ILogger logger, bool skipProfile = false) { if (FunctionLoader.FunctionAppRootPath == null) { @@ -65,6 +66,22 @@ internal PowerShellManager(ILogger logger) _pwsh.Streams.Warning.DataAdding += streamHandler.WarningDataAdding; // Initialize the Runspace + if(!skipProfile) + { + InvokeProfile(); + } + else + { + _logger.Log(LogLevel.Trace, PowerShellWorkerStrings.DeferringProfileToFirstExecution); + } + shouldInvokeProfile = skipProfile; + } + + /// + /// This method invokes the FunctionApp's profile.ps1. + /// + internal void InvokeProfile() + { InvokeProfile(FunctionLoader.FunctionAppProfilePath); } @@ -120,6 +137,12 @@ internal Hashtable InvokeFunction( try { + if (shouldInvokeProfile) + { + InvokeProfile(); + shouldInvokeProfile = false; + } + if (string.IsNullOrEmpty(entryPoint)) { _pwsh.AddCommand(scriptPath); diff --git a/src/PowerShell/PowerShellManagerPool.cs b/src/PowerShell/PowerShellManagerPool.cs index f5226434..e02c47bc 100644 --- a/src/PowerShell/PowerShellManagerPool.cs +++ b/src/PowerShell/PowerShellManagerPool.cs @@ -46,6 +46,26 @@ internal PowerShellManagerPool(MessagingStream msgStream) RpcLogger.WriteSystemLog(string.Format(PowerShellWorkerStrings.LogConcurrencyUpperBound, _upperBound.ToString())); } + /// + /// Initialize the pool and populate it with PowerShellManager instances. + /// We instantiate PowerShellManager instances in a lazy way, starting from size 1 and increase the number of workers as needed. + /// + internal void Initialize(string requestId, bool skipProfile = false) + { + var logger = new RpcLogger(_msgStream); + + try + { + logger.SetContext(requestId, invocationId: null); + _pool.Add(new PowerShellManager(logger, skipProfile)); + _poolSize = 1; + } + finally + { + logger.ResetContext(); + } + } + /// /// Checkout an idle PowerShellManager instance in a non-blocking asynchronous way. /// diff --git a/src/RequestProcessor.cs b/src/RequestProcessor.cs index 26959ad8..bbf3b48c 100644 --- a/src/RequestProcessor.cs +++ b/src/RequestProcessor.cs @@ -171,6 +171,10 @@ internal StreamingMessage ProcessFunctionLoadRequest(StreamingMessage request) // Setup the FunctionApp root path and module path. FunctionLoader.SetupWellKnownPaths(functionLoadRequest); _dependencyManager.ProcessDependencyDownload(_msgStream, request); + + // Initialize the first runspace so that the debugger has something to attach to. + // Upon the first invocation and completion of the dependencyDownload, the profile.ps1 will be run in the runspace. + _powershellPool.Initialize(request.RequestId, skipProfile: true); } catch (Exception e) { diff --git a/src/resources/PowerShellWorkerStrings.resx b/src/resources/PowerShellWorkerStrings.resx index 2af57f0c..2aa2fe84 100644 --- a/src/resources/PowerShellWorkerStrings.resx +++ b/src/resources/PowerShellWorkerStrings.resx @@ -154,6 +154,9 @@ Fail to run profile.ps1. See logs for detailed errors. Profile location: {0}. + + Deferring profile execution until first function invocation. This allows dependency management time to install all the dependencies. + Unsupported message type: {0}. diff --git a/test/Unit/PowerShell/PowerShellManagerTests.cs b/test/Unit/PowerShell/PowerShellManagerTests.cs index 26741784..d319f859 100644 --- a/test/Unit/PowerShell/PowerShellManagerTests.cs +++ b/test/Unit/PowerShell/PowerShellManagerTests.cs @@ -43,9 +43,9 @@ static TestUtils() // Have a single place to get a PowerShellManager for testing. // This is to guarantee that the well known paths are setup before calling the constructor of PowerShellManager. - internal static PowerShellManager NewTestPowerShellManager(ConsoleLogger logger) + internal static PowerShellManager NewTestPowerShellManager(ConsoleLogger logger, bool skipProfile = false) { - return new PowerShellManager(logger); + return new PowerShellManager(logger, skipProfile); } internal static AzFunctionInfo NewAzFunctionInfo(string scriptFile, string entryPoint) @@ -211,6 +211,17 @@ public void ProfileShouldWork() Assert.Equal("Information: INFORMATION: Hello PROFILE", _testLogger.FullLog[0]); } + [Fact] + public void ProfileShouldNotRunIfSkipped() + { + //initialize fresh log + _testLogger.FullLog.Clear(); + TestUtils.NewTestPowerShellManager(_testLogger, skipProfile: true); + + Assert.Single(_testLogger.FullLog); + Assert.Equal("Trace: Deferring profile execution until first function invocation. This allows dependency management time to install all the dependencies.", _testLogger.FullLog[0]); + } + [Fact] public void ProfileWithTerminatingError() { From 62925d4e9fee0365aae706b071ff5af3e9b7db31 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 29 Apr 2019 16:44:30 -0700 Subject: [PATCH 2/8] Fix regression: create the first PowerShellManager in FunctionLoad, but delay the init --- src/PowerShell/PowerShellManager.cs | 33 +++++++---------- src/PowerShell/PowerShellManagerPool.cs | 6 ++-- src/RequestProcessor.cs | 2 +- src/resources/PowerShellWorkerStrings.resx | 3 -- .../Unit/PowerShell/PowerShellManagerTests.cs | 36 ++++++++++++------- 5 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/PowerShell/PowerShellManager.cs b/src/PowerShell/PowerShellManager.cs index 147afb90..da6d3f61 100644 --- a/src/PowerShell/PowerShellManager.cs +++ b/src/PowerShell/PowerShellManager.cs @@ -22,7 +22,7 @@ internal class PowerShellManager { private readonly ILogger _logger; private readonly PowerShell _pwsh; - private bool shouldInvokeProfile; + private bool _runspaceInited; /// /// Gets the Runspace InstanceId. @@ -46,7 +46,7 @@ static PowerShellManager() addMethod.Invoke(null, new object[] { "HttpRequestContext", typeof(HttpRequestContext) }); } - internal PowerShellManager(ILogger logger, bool skipProfile = false) + internal PowerShellManager(ILogger logger, bool delayInit = false) { if (FunctionLoader.FunctionAppRootPath == null) { @@ -66,23 +66,19 @@ internal PowerShellManager(ILogger logger, bool skipProfile = false) _pwsh.Streams.Warning.DataAdding += streamHandler.WarningDataAdding; // Initialize the Runspace - if(!skipProfile) + if (!delayInit) { - InvokeProfile(); + Initialize(); } - else - { - _logger.Log(LogLevel.Trace, PowerShellWorkerStrings.DeferringProfileToFirstExecution); - } - shouldInvokeProfile = skipProfile; } - /// - /// This method invokes the FunctionApp's profile.ps1. - /// - internal void InvokeProfile() + internal void Initialize() { - InvokeProfile(FunctionLoader.FunctionAppProfilePath); + if (!_runspaceInited) + { + InvokeProfile(FunctionLoader.FunctionAppProfilePath); + _runspaceInited = true; + } } /// @@ -93,7 +89,8 @@ internal void InvokeProfile(string profilePath) Exception exception = null; if (profilePath == null) { - RpcLogger.WriteSystemLog(string.Format(PowerShellWorkerStrings.FileNotFound, "profile.ps1", FunctionLoader.FunctionAppRootPath)); + string noProfileMsg = string.Format(PowerShellWorkerStrings.FileNotFound, "profile.ps1", FunctionLoader.FunctionAppRootPath); + _logger.Log(LogLevel.Trace, noProfileMsg); return; } @@ -137,12 +134,6 @@ internal Hashtable InvokeFunction( try { - if (shouldInvokeProfile) - { - InvokeProfile(); - shouldInvokeProfile = false; - } - if (string.IsNullOrEmpty(entryPoint)) { _pwsh.AddCommand(scriptPath); diff --git a/src/PowerShell/PowerShellManagerPool.cs b/src/PowerShell/PowerShellManagerPool.cs index e02c47bc..7fec3ef5 100644 --- a/src/PowerShell/PowerShellManagerPool.cs +++ b/src/PowerShell/PowerShellManagerPool.cs @@ -50,14 +50,14 @@ internal PowerShellManagerPool(MessagingStream msgStream) /// Initialize the pool and populate it with PowerShellManager instances. /// We instantiate PowerShellManager instances in a lazy way, starting from size 1 and increase the number of workers as needed. /// - internal void Initialize(string requestId, bool skipProfile = false) + internal void Initialize(string requestId) { var logger = new RpcLogger(_msgStream); try { logger.SetContext(requestId, invocationId: null); - _pool.Add(new PowerShellManager(logger, skipProfile)); + _pool.Add(new PowerShellManager(logger, delayInit: true)); _poolSize = 1; } finally @@ -101,6 +101,8 @@ internal PowerShellManager CheckoutIdleWorker(StreamingMessage request, AzFuncti // Register the function with the Runspace before returning the idle PowerShellManager. FunctionMetadata.RegisterFunctionMetadata(psManager.InstanceId, functionInfo); psManager.Logger.SetContext(requestId, invocationId); + + psManager.Initialize(); return psManager; } diff --git a/src/RequestProcessor.cs b/src/RequestProcessor.cs index bbf3b48c..321dea59 100644 --- a/src/RequestProcessor.cs +++ b/src/RequestProcessor.cs @@ -174,7 +174,7 @@ internal StreamingMessage ProcessFunctionLoadRequest(StreamingMessage request) // Initialize the first runspace so that the debugger has something to attach to. // Upon the first invocation and completion of the dependencyDownload, the profile.ps1 will be run in the runspace. - _powershellPool.Initialize(request.RequestId, skipProfile: true); + _powershellPool.Initialize(request.RequestId); } catch (Exception e) { diff --git a/src/resources/PowerShellWorkerStrings.resx b/src/resources/PowerShellWorkerStrings.resx index 2aa2fe84..2af57f0c 100644 --- a/src/resources/PowerShellWorkerStrings.resx +++ b/src/resources/PowerShellWorkerStrings.resx @@ -154,9 +154,6 @@ Fail to run profile.ps1. See logs for detailed errors. Profile location: {0}. - - Deferring profile execution until first function invocation. This allows dependency management time to install all the dependencies. - Unsupported message type: {0}. diff --git a/test/Unit/PowerShell/PowerShellManagerTests.cs b/test/Unit/PowerShell/PowerShellManagerTests.cs index d319f859..7ddc9895 100644 --- a/test/Unit/PowerShell/PowerShellManagerTests.cs +++ b/test/Unit/PowerShell/PowerShellManagerTests.cs @@ -43,9 +43,9 @@ static TestUtils() // Have a single place to get a PowerShellManager for testing. // This is to guarantee that the well known paths are setup before calling the constructor of PowerShellManager. - internal static PowerShellManager NewTestPowerShellManager(ConsoleLogger logger, bool skipProfile = false) + internal static PowerShellManager NewTestPowerShellManager(ConsoleLogger logger, bool delayInit = false) { - return new PowerShellManager(logger, skipProfile); + return new PowerShellManager(logger, delayInit); } internal static AzFunctionInfo NewAzFunctionInfo(string scriptFile, string entryPoint) @@ -211,17 +211,6 @@ public void ProfileShouldWork() Assert.Equal("Information: INFORMATION: Hello PROFILE", _testLogger.FullLog[0]); } - [Fact] - public void ProfileShouldNotRunIfSkipped() - { - //initialize fresh log - _testLogger.FullLog.Clear(); - TestUtils.NewTestPowerShellManager(_testLogger, skipProfile: true); - - Assert.Single(_testLogger.FullLog); - Assert.Equal("Trace: Deferring profile execution until first function invocation. This allows dependency management time to install all the dependencies.", _testLogger.FullLog[0]); - } - [Fact] public void ProfileWithTerminatingError() { @@ -246,5 +235,26 @@ public void ProfileWithNonTerminatingError() Assert.Equal("Error: ERROR: help me!", _testLogger.FullLog[0]); Assert.Matches("Error: Fail to run profile.ps1. See logs for detailed errors. Profile location: ", _testLogger.FullLog[1]); } + + [Fact] + public void PSManagerCtorRunsProfileByDefault() + { + //initialize fresh log + _testLogger.FullLog.Clear(); + TestUtils.NewTestPowerShellManager(_testLogger); + + Assert.Single(_testLogger.FullLog); + Assert.Equal($"Trace: No 'profile.ps1' is found at the FunctionApp root folder: {FunctionLoader.FunctionAppRootPath}.", _testLogger.FullLog[0]); + } + + [Fact] + public void PSManagerCtorDoesNotRunProfileIfDelayInit() + { + //initialize fresh log + _testLogger.FullLog.Clear(); + TestUtils.NewTestPowerShellManager(_testLogger, delayInit: true); + + Assert.Empty(_testLogger.FullLog); + } } } From 273b5528d07019cf2e1197290c7701a65365aa78 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 29 Apr 2019 16:55:23 -0700 Subject: [PATCH 3/8] Update comments --- src/PowerShell/PowerShellManager.cs | 3 +++ src/RequestProcessor.cs | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/PowerShell/PowerShellManager.cs b/src/PowerShell/PowerShellManager.cs index da6d3f61..fcb35a9a 100644 --- a/src/PowerShell/PowerShellManager.cs +++ b/src/PowerShell/PowerShellManager.cs @@ -72,6 +72,9 @@ internal PowerShellManager(ILogger logger, bool delayInit = false) } } + /// + /// Extra initialization of the Runspace. + /// internal void Initialize() { if (!_runspaceInited) diff --git a/src/RequestProcessor.cs b/src/RequestProcessor.cs index 321dea59..81c4f62d 100644 --- a/src/RequestProcessor.cs +++ b/src/RequestProcessor.cs @@ -172,8 +172,9 @@ internal StreamingMessage ProcessFunctionLoadRequest(StreamingMessage request) FunctionLoader.SetupWellKnownPaths(functionLoadRequest); _dependencyManager.ProcessDependencyDownload(_msgStream, request); - // Initialize the first runspace so that the debugger has something to attach to. - // Upon the first invocation and completion of the dependencyDownload, the profile.ps1 will be run in the runspace. + // Create the first Runspace so that the debugger has the target to attach to. + // The further initialization of the Runspace (e.g. invoking profile.ps1) is delayed until + // the first invocation and completion of the dependency download. _powershellPool.Initialize(request.RequestId); } catch (Exception e) From 8124505900ed55b22c5361d4f42057aa0e61d001 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 30 Apr 2019 21:50:25 -0700 Subject: [PATCH 4/8] Minor update --- src/PowerShell/PowerShellManagerPool.cs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/PowerShell/PowerShellManagerPool.cs b/src/PowerShell/PowerShellManagerPool.cs index 7fec3ef5..c4061614 100644 --- a/src/PowerShell/PowerShellManagerPool.cs +++ b/src/PowerShell/PowerShellManagerPool.cs @@ -52,18 +52,8 @@ internal PowerShellManagerPool(MessagingStream msgStream) /// internal void Initialize(string requestId) { - var logger = new RpcLogger(_msgStream); - - try - { - logger.SetContext(requestId, invocationId: null); - _pool.Add(new PowerShellManager(logger, delayInit: true)); - _poolSize = 1; - } - finally - { - logger.ResetContext(); - } + _pool.Add(new PowerShellManager(new RpcLogger(_msgStream), delayInit: true)); + _poolSize = 1; } /// @@ -98,11 +88,14 @@ internal PowerShellManager CheckoutIdleWorker(StreamingMessage request, AzFuncti } } + // Finish the initialization if not yet. + // This applies only to the very first PowerShellManager instance, whose initialization was deferred. + psManager.Initialize(); + // Register the function with the Runspace before returning the idle PowerShellManager. FunctionMetadata.RegisterFunctionMetadata(psManager.InstanceId, functionInfo); psManager.Logger.SetContext(requestId, invocationId); - psManager.Initialize(); return psManager; } From f7314dfefac04b64652f22f784a100aac24ce69e Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 30 Apr 2019 21:55:49 -0700 Subject: [PATCH 5/8] Remove unused parameter --- src/PowerShell/PowerShellManagerPool.cs | 2 +- src/RequestProcessor.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PowerShell/PowerShellManagerPool.cs b/src/PowerShell/PowerShellManagerPool.cs index c4061614..35406249 100644 --- a/src/PowerShell/PowerShellManagerPool.cs +++ b/src/PowerShell/PowerShellManagerPool.cs @@ -50,7 +50,7 @@ internal PowerShellManagerPool(MessagingStream msgStream) /// Initialize the pool and populate it with PowerShellManager instances. /// We instantiate PowerShellManager instances in a lazy way, starting from size 1 and increase the number of workers as needed. /// - internal void Initialize(string requestId) + internal void Initialize() { _pool.Add(new PowerShellManager(new RpcLogger(_msgStream), delayInit: true)); _poolSize = 1; diff --git a/src/RequestProcessor.cs b/src/RequestProcessor.cs index 81c4f62d..278e5766 100644 --- a/src/RequestProcessor.cs +++ b/src/RequestProcessor.cs @@ -175,7 +175,7 @@ internal StreamingMessage ProcessFunctionLoadRequest(StreamingMessage request) // Create the first Runspace so that the debugger has the target to attach to. // The further initialization of the Runspace (e.g. invoking profile.ps1) is delayed until // the first invocation and completion of the dependency download. - _powershellPool.Initialize(request.RequestId); + _powershellPool.Initialize(); } catch (Exception e) { From cc0a960ea63381396905fb8c127f7493370903e0 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 3 May 2019 14:59:07 -0700 Subject: [PATCH 6/8] Refactor the code to make dependency manager use the same PowerShell instance for downloading --- src/DependencyManagement/DependencyManager.cs | 23 ++----- src/PowerShell/PowerShellManager.cs | 53 ++++++++++------ src/PowerShell/PowerShellManagerPool.cs | 6 +- src/RequestProcessor.cs | 13 ++-- src/Utility/Utils.cs | 63 +++++++++++++------ .../Unit/PowerShell/PowerShellManagerTests.cs | 10 +-- 6 files changed, 98 insertions(+), 70 deletions(-) diff --git a/src/DependencyManagement/DependencyManager.cs b/src/DependencyManagement/DependencyManager.cs index 112ed48f..8210e89d 100644 --- a/src/DependencyManagement/DependencyManager.cs +++ b/src/DependencyManagement/DependencyManager.cs @@ -79,7 +79,8 @@ internal DependencyManager() /// /// The protobuf messaging stream /// The StreamingMessage request for function load - internal void ProcessDependencyDownload(MessagingStream msgStream, StreamingMessage request) + /// The PowerShell instance used to download modules + internal void ProcessDependencyDownload(MessagingStream msgStream, StreamingMessage request, PowerShell pwsh) { if (request.FunctionLoadRequest.ManagedDependencyEnabled) { @@ -100,7 +101,7 @@ internal void ProcessDependencyDownload(MessagingStream msgStream, StreamingMess } //Start dependency download on a separate thread - _dependencyDownloadTask = Task.Run(() => ProcessDependencies(rpcLogger)); + _dependencyDownloadTask = Task.Run(() => InstallFunctionAppDependencies(pwsh, rpcLogger)); } } @@ -117,22 +118,6 @@ internal void WaitOnDependencyDownload() } } - private void ProcessDependencies(RpcLogger rpcLogger) - { - try - { - _dependencyError = null; - using (PowerShell pwsh = PowerShell.Create(Utils.SingletonISS.Value)) - { - InstallFunctionAppDependencies(pwsh, rpcLogger); - } - } - catch (Exception e) - { - _dependencyError = e; - } - } - /// /// Initializes the dependency manger and performs the following: /// - Parse functionAppRoot\requirements.psd1 file and create a list of dependencies to install. @@ -251,7 +236,7 @@ internal void InstallFunctionAppDependencies(PowerShell pwsh, ILogger logger) catch (Exception e) { var errorMsg = string.Format(PowerShellWorkerStrings.FailToInstallFuncAppDependencies, e.Message); - throw new DependencyInstallationException(errorMsg, e); + _dependencyError = new DependencyInstallationException(errorMsg, e); } } diff --git a/src/PowerShell/PowerShellManager.cs b/src/PowerShell/PowerShellManager.cs index fcb35a9a..cc52cd29 100644 --- a/src/PowerShell/PowerShellManager.cs +++ b/src/PowerShell/PowerShellManager.cs @@ -46,30 +46,28 @@ static PowerShellManager() addMethod.Invoke(null, new object[] { "HttpRequestContext", typeof(HttpRequestContext) }); } - internal PowerShellManager(ILogger logger, bool delayInit = false) + /// + /// Create a PowerShellManager instance but defer the Initialization. + /// + /// + /// This constructor is only for creating the very first PowerShellManager instance. + /// The initialization work is deferred until all prerequisites are ready, such as + /// the dependent modules are downloaded and all Az functions are loaded. + /// + internal PowerShellManager(ILogger logger, PowerShell pwsh) { - if (FunctionLoader.FunctionAppRootPath == null) - { - throw new InvalidOperationException(PowerShellWorkerStrings.FunctionAppRootNotResolved); - } - _logger = logger; - _pwsh = PowerShell.Create(Utils.SingletonISS.Value); - - // Setup Stream event listeners - var streamHandler = new StreamHandler(logger); - _pwsh.Streams.Debug.DataAdding += streamHandler.DebugDataAdding; - _pwsh.Streams.Error.DataAdding += streamHandler.ErrorDataAdding; - _pwsh.Streams.Information.DataAdding += streamHandler.InformationDataAdding; - _pwsh.Streams.Progress.DataAdding += streamHandler.ProgressDataAdding; - _pwsh.Streams.Verbose.DataAdding += streamHandler.VerboseDataAdding; - _pwsh.Streams.Warning.DataAdding += streamHandler.WarningDataAdding; + _pwsh = pwsh; + } + /// + /// Create a PowerShellManager instance and initialize it. + /// + internal PowerShellManager(ILogger logger) + : this(logger, Utils.NewPwshInstance()) + { // Initialize the Runspace - if (!delayInit) - { - Initialize(); - } + Initialize(); } /// @@ -79,11 +77,26 @@ internal void Initialize() { if (!_runspaceInited) { + RegisterStreamEvents(); InvokeProfile(FunctionLoader.FunctionAppProfilePath); _runspaceInited = true; } } + /// + /// Setup Stream event listeners. + /// + private void RegisterStreamEvents() + { + var streamHandler = new StreamHandler(_logger); + _pwsh.Streams.Debug.DataAdding += streamHandler.DebugDataAdding; + _pwsh.Streams.Error.DataAdding += streamHandler.ErrorDataAdding; + _pwsh.Streams.Information.DataAdding += streamHandler.InformationDataAdding; + _pwsh.Streams.Progress.DataAdding += streamHandler.ProgressDataAdding; + _pwsh.Streams.Verbose.DataAdding += streamHandler.VerboseDataAdding; + _pwsh.Streams.Warning.DataAdding += streamHandler.WarningDataAdding; + } + /// /// This method invokes the FunctionApp's profile.ps1. /// diff --git a/src/PowerShell/PowerShellManagerPool.cs b/src/PowerShell/PowerShellManagerPool.cs index 35406249..6f69bbbe 100644 --- a/src/PowerShell/PowerShellManagerPool.cs +++ b/src/PowerShell/PowerShellManagerPool.cs @@ -50,9 +50,11 @@ internal PowerShellManagerPool(MessagingStream msgStream) /// Initialize the pool and populate it with PowerShellManager instances. /// We instantiate PowerShellManager instances in a lazy way, starting from size 1 and increase the number of workers as needed. /// - internal void Initialize() + internal void Initialize(PowerShell pwsh) { - _pool.Add(new PowerShellManager(new RpcLogger(_msgStream), delayInit: true)); + var logger = new RpcLogger(_msgStream); + var psManager = new PowerShellManager(logger, pwsh); + _pool.Add(psManager); _poolSize = 1; } diff --git a/src/RequestProcessor.cs b/src/RequestProcessor.cs index 278e5766..f0d37245 100644 --- a/src/RequestProcessor.cs +++ b/src/RequestProcessor.cs @@ -170,12 +170,15 @@ internal StreamingMessage ProcessFunctionLoadRequest(StreamingMessage request) // Setup the FunctionApp root path and module path. FunctionLoader.SetupWellKnownPaths(functionLoadRequest); - _dependencyManager.ProcessDependencyDownload(_msgStream, request); - // Create the first Runspace so that the debugger has the target to attach to. - // The further initialization of the Runspace (e.g. invoking profile.ps1) is delayed until - // the first invocation and completion of the dependency download. - _powershellPool.Initialize(); + // Create the very first Runspace so the debugger has the target to attach to. + // This PowerShell instance is shared by the first PowerShellManager instance created in the pool, + // and the dependency manager (used to download dependent modules if needed). + var pwsh = Utils.NewPwshInstance(); + _powershellPool.Initialize(pwsh); + + // Start the download asynchronously if needed. + _dependencyManager.ProcessDependencyDownload(_msgStream, request, pwsh); } catch (Exception e) { diff --git a/src/Utility/Utils.cs b/src/Utility/Utils.cs index 5b94bc0b..ff3860e0 100644 --- a/src/Utility/Utils.cs +++ b/src/Utility/Utils.cs @@ -5,14 +5,15 @@ using System; using System.IO; -using System.Management.Automation; -using System.Management.Automation.Runspaces; using System.Text; using System.Threading; using Microsoft.PowerShell.Commands; namespace Microsoft.Azure.Functions.PowerShellWorker.Utility { + using System.Management.Automation; + using System.Management.Automation.Runspaces; + internal class Utils { internal readonly static CmdletInfo ImportModuleCmdletInfo = new CmdletInfo("Import-Module", typeof(ImportModuleCommand)); @@ -20,28 +21,50 @@ internal class Utils internal readonly static CmdletInfo GetJobCmdletInfo = new CmdletInfo("Get-Job", typeof(GetJobCommand)); internal readonly static CmdletInfo RemoveJobCmdletInfo = new CmdletInfo("Remove-Job", typeof(RemoveJobCommand)); - internal readonly static Lazy SingletonISS - = new Lazy(NewInitialSessionState, LazyThreadSafetyMode.PublicationOnly); - - private static InitialSessionState NewInitialSessionState() + private static InitialSessionState s_iss; + private static InitialSessionState SingletonISS { - var iss = InitialSessionState.CreateDefault(); - iss.ThreadOptions = PSThreadOptions.UseCurrentThread; - iss.EnvironmentVariables.Add( - new SessionStateVariableEntry( - "PSModulePath", - FunctionLoader.FunctionModulePath, - description: null)); - - // Setting the execution policy on macOS and Linux throws an exception so only update it on Windows - if(Platform.IsWindows) + get { - // This sets the execution policy on Windows to Unrestricted which is required to run the user's function scripts on - // Windows client versions. This is needed if a user is testing their function locally with the func CLI. - iss.ExecutionPolicy = Microsoft.PowerShell.ExecutionPolicy.Unrestricted; + if (s_iss != null) + { + return s_iss; + } + + if (FunctionLoader.FunctionAppRootPath == null) + { + throw new InvalidOperationException(PowerShellWorkerStrings.FunctionAppRootNotResolved); + } + + s_iss = InitialSessionState.CreateDefault(); + s_iss.ThreadOptions = PSThreadOptions.UseCurrentThread; + s_iss.EnvironmentVariables.Add( + new SessionStateVariableEntry( + "PSModulePath", + FunctionLoader.FunctionModulePath, + description: null)); + + // Setting the execution policy on macOS and Linux throws an exception so only update it on Windows + if(Platform.IsWindows) + { + // This sets the execution policy on Windows to Unrestricted which is required to run the user's function scripts on + // Windows client versions. This is needed if a user is testing their function locally with the func CLI. + s_iss.ExecutionPolicy = Microsoft.PowerShell.ExecutionPolicy.Unrestricted; + } + + return s_iss; } + } + + /// + /// Create a new PowerShell instance using our singleton InitialSessionState instance. + /// + internal static PowerShell NewPwshInstance() + { + var pwsh = PowerShell.Create(SingletonISS); + pwsh.Runspace.Name = "PowerShellManager"; - return iss; + return pwsh; } /// diff --git a/test/Unit/PowerShell/PowerShellManagerTests.cs b/test/Unit/PowerShell/PowerShellManagerTests.cs index 7ddc9895..5fe06915 100644 --- a/test/Unit/PowerShell/PowerShellManagerTests.cs +++ b/test/Unit/PowerShell/PowerShellManagerTests.cs @@ -7,13 +7,15 @@ using System.Collections; using System.Collections.Generic; using System.IO; -using System.Management.Automation; using Microsoft.Azure.Functions.PowerShellWorker.PowerShell; +using Microsoft.Azure.Functions.PowerShellWorker.Utility; using Microsoft.Azure.WebJobs.Script.Grpc.Messages; using Xunit; namespace Microsoft.Azure.Functions.PowerShellWorker.Test { + using System.Management.Automation; + internal class TestUtils { internal const string TestInputBindingName = "req"; @@ -43,9 +45,9 @@ static TestUtils() // Have a single place to get a PowerShellManager for testing. // This is to guarantee that the well known paths are setup before calling the constructor of PowerShellManager. - internal static PowerShellManager NewTestPowerShellManager(ConsoleLogger logger, bool delayInit = false) + internal static PowerShellManager NewTestPowerShellManager(ConsoleLogger logger, PowerShell pwsh = null) { - return new PowerShellManager(logger, delayInit); + return pwsh != null ? new PowerShellManager(logger, pwsh) : new PowerShellManager(logger); } internal static AzFunctionInfo NewAzFunctionInfo(string scriptFile, string entryPoint) @@ -252,7 +254,7 @@ public void PSManagerCtorDoesNotRunProfileIfDelayInit() { //initialize fresh log _testLogger.FullLog.Clear(); - TestUtils.NewTestPowerShellManager(_testLogger, delayInit: true); + TestUtils.NewTestPowerShellManager(_testLogger, Utils.NewPwshInstance()); Assert.Empty(_testLogger.FullLog); } From 6cfb0c7ea42ea3cfff6a729554dbe0a260b8bec2 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Sat, 4 May 2019 10:39:14 -0700 Subject: [PATCH 7/8] Label the runspace name with id --- src/PowerShell/PowerShellManager.cs | 17 +++++++++--- src/PowerShell/PowerShellManagerPool.cs | 22 +++++++++------- src/Utility/Utils.cs | 26 +++++-------------- .../Unit/PowerShell/PowerShellManagerTests.cs | 2 +- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/PowerShell/PowerShellManager.cs b/src/PowerShell/PowerShellManager.cs index cc52cd29..19ec6f19 100644 --- a/src/PowerShell/PowerShellManager.cs +++ b/src/PowerShell/PowerShellManager.cs @@ -46,6 +46,16 @@ static PowerShellManager() addMethod.Invoke(null, new object[] { "HttpRequestContext", typeof(HttpRequestContext) }); } + /// + /// Constructor for setting the basic fields. + /// + private PowerShellManager(ILogger logger, PowerShell pwsh, int id) + { + _logger = logger; + _pwsh = pwsh; + _pwsh.Runspace.Name = $"PowerShellManager{id}"; + } + /// /// Create a PowerShellManager instance but defer the Initialization. /// @@ -55,16 +65,15 @@ static PowerShellManager() /// the dependent modules are downloaded and all Az functions are loaded. /// internal PowerShellManager(ILogger logger, PowerShell pwsh) + : this(logger, pwsh, id: 1) { - _logger = logger; - _pwsh = pwsh; } /// /// Create a PowerShellManager instance and initialize it. /// - internal PowerShellManager(ILogger logger) - : this(logger, Utils.NewPwshInstance()) + internal PowerShellManager(ILogger logger, int id) + : this(logger, Utils.NewPwshInstance(), id) { // Initialize the Runspace Initialize(); diff --git a/src/PowerShell/PowerShellManagerPool.cs b/src/PowerShell/PowerShellManagerPool.cs index 6f69bbbe..4ddaf898 100644 --- a/src/PowerShell/PowerShellManagerPool.cs +++ b/src/PowerShell/PowerShellManagerPool.cs @@ -71,18 +71,22 @@ internal PowerShellManager CheckoutIdleWorker(StreamingMessage request, AzFuncti if (!_pool.TryTake(out psManager)) { // The pool doesn't have an idle one. - if (_poolSize < _upperBound && - Interlocked.Increment(ref _poolSize) <= _upperBound) + if (_poolSize < _upperBound) { - // If the pool hasn't reached its bounded capacity yet, then - // we create a new item and return it. - var logger = new RpcLogger(_msgStream); - logger.SetContext(requestId, invocationId); - psManager = new PowerShellManager(logger); + int id = Interlocked.Increment(ref _poolSize); + if (id <= _upperBound) + { + // If the pool hasn't reached its bounded capacity yet, then + // we create a new item and return it. + var logger = new RpcLogger(_msgStream); + logger.SetContext(requestId, invocationId); + psManager = new PowerShellManager(logger, id); - RpcLogger.WriteSystemLog(string.Format(PowerShellWorkerStrings.LogNewPowerShellManagerCreated, _poolSize.ToString())); + RpcLogger.WriteSystemLog(string.Format(PowerShellWorkerStrings.LogNewPowerShellManagerCreated, id.ToString())); + } } - else + + if (psManager == null) { // If the pool has reached its bounded capacity, then the thread // should be blocked until an idle one becomes available. diff --git a/src/Utility/Utils.cs b/src/Utility/Utils.cs index ff3860e0..349bbe5d 100644 --- a/src/Utility/Utils.cs +++ b/src/Utility/Utils.cs @@ -22,15 +22,14 @@ internal class Utils internal readonly static CmdletInfo RemoveJobCmdletInfo = new CmdletInfo("Remove-Job", typeof(RemoveJobCommand)); private static InitialSessionState s_iss; - private static InitialSessionState SingletonISS + + /// + /// Create a new PowerShell instance using our singleton InitialSessionState instance. + /// + internal static PowerShell NewPwshInstance() { - get + if (s_iss == null) { - if (s_iss != null) - { - return s_iss; - } - if (FunctionLoader.FunctionAppRootPath == null) { throw new InvalidOperationException(PowerShellWorkerStrings.FunctionAppRootNotResolved); @@ -51,20 +50,9 @@ private static InitialSessionState SingletonISS // Windows client versions. This is needed if a user is testing their function locally with the func CLI. s_iss.ExecutionPolicy = Microsoft.PowerShell.ExecutionPolicy.Unrestricted; } - - return s_iss; } - } - - /// - /// Create a new PowerShell instance using our singleton InitialSessionState instance. - /// - internal static PowerShell NewPwshInstance() - { - var pwsh = PowerShell.Create(SingletonISS); - pwsh.Runspace.Name = "PowerShellManager"; - return pwsh; + return PowerShell.Create(s_iss); } /// diff --git a/test/Unit/PowerShell/PowerShellManagerTests.cs b/test/Unit/PowerShell/PowerShellManagerTests.cs index 5fe06915..75e5a3fb 100644 --- a/test/Unit/PowerShell/PowerShellManagerTests.cs +++ b/test/Unit/PowerShell/PowerShellManagerTests.cs @@ -47,7 +47,7 @@ static TestUtils() // This is to guarantee that the well known paths are setup before calling the constructor of PowerShellManager. internal static PowerShellManager NewTestPowerShellManager(ConsoleLogger logger, PowerShell pwsh = null) { - return pwsh != null ? new PowerShellManager(logger, pwsh) : new PowerShellManager(logger); + return pwsh != null ? new PowerShellManager(logger, pwsh) : new PowerShellManager(logger, id: 2); } internal static AzFunctionInfo NewAzFunctionInfo(string scriptFile, string entryPoint) From 9dc2fa268e70d2f814458ff45bad270bfd1d1c12 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 6 May 2019 14:03:37 -0700 Subject: [PATCH 8/8] Update a comment --- src/PowerShell/PowerShellManagerPool.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PowerShell/PowerShellManagerPool.cs b/src/PowerShell/PowerShellManagerPool.cs index 4ddaf898..d2fb13ab 100644 --- a/src/PowerShell/PowerShellManagerPool.cs +++ b/src/PowerShell/PowerShellManagerPool.cs @@ -47,7 +47,7 @@ internal PowerShellManagerPool(MessagingStream msgStream) } /// - /// Initialize the pool and populate it with PowerShellManager instances. + /// Populate the pool with the very first PowerShellManager instance. /// We instantiate PowerShellManager instances in a lazy way, starting from size 1 and increase the number of workers as needed. /// internal void Initialize(PowerShell pwsh)