From ad7278bbff6c0c57695af4062ffbcab67f6be79b Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 21 Mar 2024 16:25:12 -0700 Subject: [PATCH] Make sure the assembly/library resolvers are registered at early stage (#21361) --- .../CoreCLR/CorePsAssemblyLoadContext.cs | 12 ++++++---- .../engine/hostifaces/ConnectionBase.cs | 22 +++++++++++++++++++ .../utils/ClrFacade.cs | 16 +++++++++++--- .../Basic/RegisterAssemblyResolverEarly.ps1 | 13 +++++++++++ 4 files changed, 56 insertions(+), 7 deletions(-) create mode 100644 test/powershell/engine/Basic/RegisterAssemblyResolverEarly.ps1 diff --git a/src/System.Management.Automation/CoreCLR/CorePsAssemblyLoadContext.cs b/src/System.Management.Automation/CoreCLR/CorePsAssemblyLoadContext.cs index 0aba1eddfe..6be8d3c595 100644 --- a/src/System.Management.Automation/CoreCLR/CorePsAssemblyLoadContext.cs +++ b/src/System.Management.Automation/CoreCLR/CorePsAssemblyLoadContext.cs @@ -36,16 +36,19 @@ namespace System.Management.Automation /// /// Initialize a singleton of PowerShellAssemblyLoadContext. /// - internal static PowerShellAssemblyLoadContext InitializeSingleton(string basePaths) + internal static PowerShellAssemblyLoadContext InitializeSingleton(string basePaths, bool throwOnReentry) { lock (s_syncObj) { - if (Instance != null) + if (Instance is null) + { + Instance = new PowerShellAssemblyLoadContext(basePaths); + } + else if (throwOnReentry) { throw new InvalidOperationException(SingletonAlreadyInitialized); } - Instance = new PowerShellAssemblyLoadContext(basePaths); return Instance; } } @@ -581,7 +584,8 @@ namespace System.Management.Automation { ArgumentException.ThrowIfNullOrEmpty(basePaths); - PowerShellAssemblyLoadContext.InitializeSingleton(basePaths); + // Disallow calling this method from native code for more than once. + PowerShellAssemblyLoadContext.InitializeSingleton(basePaths, throwOnReentry: true); } } diff --git a/src/System.Management.Automation/engine/hostifaces/ConnectionBase.cs b/src/System.Management.Automation/engine/hostifaces/ConnectionBase.cs index 23daa3c871..0969d7b08c 100644 --- a/src/System.Management.Automation/engine/hostifaces/ConnectionBase.cs +++ b/src/System.Management.Automation/engine/hostifaces/ConnectionBase.cs @@ -25,6 +25,28 @@ namespace System.Management.Automation.Runspaces { #region constructors + /// + /// Initialize powershell AssemblyLoadContext and register the 'Resolving' event, if it's not done already. + /// If powershell is hosted by a native host such as DSC, then PS ALC may be initialized via 'SetPowerShellAssemblyLoadContext' before loading S.M.A. + /// + /// + /// We do this both here and during the initialization of the 'ClrFacade' type. + /// This is because we want to make sure the assembly/library resolvers are: + /// 1. registered before any script/cmdlet can run. + /// 2. registered before 'ClrFacade' gets used for assembly related operations. + /// + /// The 'ClrFacade' type may be used without a Runspace created, for example, by calling type conversion methods in the 'LanguagePrimitive' type. + /// And at the mean time, script or cmdlet may run without the 'ClrFacade' type initialized. + /// That's why we attempt to create the singleton of 'PowerShellAssemblyLoadContext' at both places. + /// + static RunspaceBase() + { + if (PowerShellAssemblyLoadContext.Instance is null) + { + PowerShellAssemblyLoadContext.InitializeSingleton(string.Empty, throwOnReentry: false); + } + } + /// /// Construct an instance of an Runspace using a custom /// implementation of PSHost. diff --git a/src/System.Management.Automation/utils/ClrFacade.cs b/src/System.Management.Automation/utils/ClrFacade.cs index aa99773435..058c33a68b 100644 --- a/src/System.Management.Automation/utils/ClrFacade.cs +++ b/src/System.Management.Automation/utils/ClrFacade.cs @@ -22,13 +22,23 @@ namespace System.Management.Automation { /// /// Initialize powershell AssemblyLoadContext and register the 'Resolving' event, if it's not done already. - /// If powershell is hosted by a native host such as DSC, then PS ALC might be initialized via 'SetPowerShellAssemblyLoadContext' before loading S.M.A. + /// If powershell is hosted by a native host such as DSC, then PS ALC may be initialized via 'SetPowerShellAssemblyLoadContext' before loading S.M.A. /// + /// + /// We do this both here and during the initialization of the 'RunspaceBase' type. + /// This is because we want to make sure the assembly/library resolvers are: + /// 1. registered before any script/cmdlet can run. + /// 2. registered before 'ClrFacade' gets used for assembly related operations. + /// + /// The 'ClrFacade' type may be used without a Runspace created, for example, by calling type conversion methods in the 'LanguagePrimitive' type. + /// And at the mean time, script or cmdlet may run without the 'ClrFacade' type initialized. + /// That's why we attempt to create the singleton of 'PowerShellAssemblyLoadContext' at both places. + /// static ClrFacade() { - if (PowerShellAssemblyLoadContext.Instance == null) + if (PowerShellAssemblyLoadContext.Instance is null) { - PowerShellAssemblyLoadContext.InitializeSingleton(string.Empty); + PowerShellAssemblyLoadContext.InitializeSingleton(string.Empty, throwOnReentry: false); } } diff --git a/test/powershell/engine/Basic/RegisterAssemblyResolverEarly.ps1 b/test/powershell/engine/Basic/RegisterAssemblyResolverEarly.ps1 new file mode 100644 index 0000000000..fbd745b3e1 --- /dev/null +++ b/test/powershell/engine/Basic/RegisterAssemblyResolverEarly.ps1 @@ -0,0 +1,13 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe "Assembly resolvers should be registered early at startup" -Tags "CI" { + + ## The PKI module requires loading an assembly from GAC, and thus depends on the PowerShell assembly resolver to work. + It "Can load the PKI module with 'pwsh -ExecutionPolicy bypass -NoProfile -c `"Import-Module PKI`"'" -Skip:(!$IsWindows) { + ## Use 'Bypass' execution policy so that it doesn't trigger 'AuthorizationManager' which would trigger 'ClrFacade' initialization. + ## We want to make sure even if 'ClrFacade' is not hit during startup, the resolvers are still registered early enough. + $out = pwsh -ExecutionPolicy bypass -NoProfile -c "Import-Module PKI; Get-Module | % name" + $out | Should -BeExactly "PKI" + } +}