Fix a module-loading regression that caused an infinite loop (#6843)

This regression was introduced by #6523, in `PSModuleInfo.cs`. A circular nested module check was removed because the comment there suggested it happens only with a deprecated workflow module. This causes a `StackOverflow` exception when running into circular nested modules.

Circular nested modules could happen for a module that is not well structured. For example, the module folder `test` contains two files: `test.psd1` and `test.psm1`, and `test.psd1` has the following content:
   @{ ModuleVersion = '0.0.1'; RootModule = 'test'; NestedModules = @('test') }

The same value `test` is put in both RootModule and NestedModule, which will end up with a module whose nested module points to itself.

There are two changes in this PR:
1. Add back the check for circular nested modules in `PSModuleInfo.cs`.
2. Remove a wrong `Dbg.Assert` in `ModuleCmdletBase.cs` and two checks before it.
   - For the assertion `Dbg.Assert(newManifestInfo.SessionState == ss`, when facing the example above, the nested module will first be loaded with a different session state, and then when trying to load the root module, the same loaded nested module will be reused for it. So 'newManifestInfo.SessionState' is not `ss`. The assertion will fail in that case.
   - For the two checks before the assertion, they are not needed anymore based on the comments there.
This commit is contained in:
Dongbo Wang
2018-05-10 16:09:36 -07:00
committed by GitHub
parent 4af9f4edac
commit db45785341
3 changed files with 43 additions and 27 deletions
@@ -2820,7 +2820,7 @@ namespace Microsoft.PowerShell.Commands
{
bool found = false;
// Never load nested modules to the global scope.
bool oldGLobal = this.BaseGlobal;
bool oldGlobal = this.BaseGlobal;
this.BaseGlobal = false;
PSModuleInfo nestedModule = LoadModuleNamedInManifest(
@@ -2838,7 +2838,7 @@ namespace Microsoft.PowerShell.Commands
out found,
shortModuleName: null);
this.BaseGlobal = oldGLobal;
this.BaseGlobal = oldGlobal;
// If found, add it to the parent's list of NestedModules
if (found)
@@ -2976,30 +2976,16 @@ namespace Microsoft.PowerShell.Commands
BaseCmdletPatterns = oldCmdletPatterns;
}
// If there is an existing session state and the new module info
// session state is empty, then use the existing session state.
// This will be the case when ModuleToProcess is a binary module
// and there were nested modules.
if (newManifestInfo.SessionState == null && ss != null)
{
newManifestInfo.SessionState = ss;
ss.Internal.Module = newManifestInfo;
}
// If the new module info session state is not empty but there is no
// existing session state, then use the new module info's sessions state;
// This will be the case if the module to process is a script module
// but there where no nested modules.
else if (newManifestInfo.SessionState != null && ss == null)
{
ss = newManifestInfo.SessionState;
}
// We don't need to care if they are both null which will be the
// case when there is only a binary module in ModuleToProcess and
// no nested modules. At this point the moduleInfo and the value in
// ss should be identical.
Dbg.Assert(newManifestInfo.SessionState == ss,
"ss and newManifestInfo.SessionState should be the same after handling ModuleToProcess");
// For most cases, 'newManifestInfo.SessionState' should be identical to 'ss':
// 1. when 'importingModule == true', 'newManifestInfo' uses the same session state as 'ss' because we passed in 'ss' when loading the RootModule via 'LoadModuleNamedInManifest'.
// 2. when 'importingModule == false', both session states will be null since we are in module analysis mode (Get-Module -ListAvailable).
//
// However, there is one exception when the RootModule is also put in NestedModules in the module manifest (ill-organized module structure).
// For example, module folder 'test' contains two files: 'test.psd1' and 'test.psm1', and 'test.psd1' has the following content:
// "@{ ModuleVersion = '0.0.1'; RootModule = 'test'; NestedModules = @('test') }"
//
// In that case, the nested module will first be loaded with a different session state, and then when trying to load the RootModule via 'LoadModuleNamedInManifest',
// the same loaded nested module will be reused for the RootModule by 'LoadModuleNamedInManifest'.
// Change the module name to match the manifest name, not the original name
newManifestInfo.SetName(manifestInfo.Name);
@@ -3248,7 +3234,6 @@ namespace Microsoft.PowerShell.Commands
{
if ((exportedCmdlets != null) && (ss != null))
{
Dbg.Assert(ss.Internal.ExportedCmdlets != null, "ss.Internal.ExportedCmdlets should not be null");
manifestInfo.ExportedCmdlets.Clear();
// Mark stuff for export
@@ -577,6 +577,15 @@ namespace System.Management.Automation
var res = new Dictionary<string, TypeDefinitionAst>(StringComparer.OrdinalIgnoreCase);
foreach (var nestedModule in this.NestedModules)
{
if (nestedModule == this)
{
// Circular nested modules could happen with ill-organized module structure.
// For example, module folder 'test' has two files: 'test.psd1' and 'test.psm1', and 'test.psd1' has the following content:
// "@{ ModuleVersion = '0.0.1'; RootModule = 'test'; NestedModules = @('test') }"
// Then, 'Import-Module test.psd1 -PassThru' will return a ModuleInfo object with circular nested modules.
continue;
}
foreach (var typePairs in nestedModule.GetExportedTypeDefinitions())
{
// The last one name wins! It's the same for command names in nested modules.
@@ -241,3 +241,25 @@ Describe "Workflow .Xaml module is not supported in PSCore" -tags "Feature" {
{ Import-Module $xamlNestedModule -ErrorAction Stop } | Should -Throw -ErrorId "Modules_WorkflowModuleNotSupported,Microsoft.PowerShell.Commands.ImportModuleCommand"
}
}
Describe "Circular nested module test" -tags "CI" {
BeforeAll {
$moduleFolder = Join-Path $TestDrive CircularNestedModuleTest
$psdPath = Join-Path $moduleFolder CircularNestedModuleTest.psd1
$psmPath = Join-Path $moduleFolder CircularNestedModuleTest.psm1
New-Item -Path $moduleFolder -ItemType Directory -Force > $null
Set-Content -Path $psdPath -Value "@{ ModuleVersion = '0.0.1'; RootModule = 'CircularNestedModuleTest'; NestedModules = @('CircularNestedModuleTest') }" -Encoding Ascii
Set-Content -Path $psmPath -Value "function bar {}" -Encoding Ascii
}
AfterAll {
Remove-Module -Name CircularNestedModuleTest -Force -ErrorAction SilentlyContinue
Remove-Item -Path $moduleFolder -Force -Recurse
}
It "Loading the module should succeed and return a module with circular nested module" {
$m = Import-Module $psdPath -PassThru
$m.NestedModules[0] | Should -Be $m
}
}