Change CommandInfoCache to implement IDisposable and clean up the runspace pool (#1335)

* Change helper to implement IDisposable and clean up the runspace pool

The Helper is recreated with every invocation of Invoke-ScriptAnalyzer which essentially resulted in a runspace which was not cleaned up for every invocation.
I saw 100s of runspaces (ia get-runspace) in my session. I also gave the CommandInfoCache it's own runspace pool and implemented IDisposable in it.
Lastly, I've added a test to catch this in the future.

* Remove runspace from helper

It no longer needs to be IDisposable
Implement dispose in cache along suggested guidelines

* Update Tests/Engine/Helper.tests.ps1

Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]>

* Update Tests/Engine/Helper.tests.ps1

Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]>

* Update Tests/Engine/Helper.tests.ps1

Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]>

* update test language

* Update Tests/Engine/Helper.tests.ps1

Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]>

* Fix version comparison to just use major number

Author: JamesWTruher

Engine/CommandInfoCache.cs

@@ -12,20 +12,44 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer
     /// <summary>
     /// Provides threadsafe caching around CommandInfo lookups with `Get-Command -Name ...`.
     /// </summary>
-    internal class CommandInfoCache
+    internal class CommandInfoCache : IDisposable
     {
         private readonly ConcurrentDictionary<CommandLookupKey, Lazy<CommandInfo>> _commandInfoCache;
         private readonly Helper _helperInstance;
         private readonly RunspacePool _runspacePool;
+        private bool disposed = false;
 
         /// <summary>
         /// Create a fresh command info cache instance.
         /// </summary>
-        public CommandInfoCache(Helper pssaHelperInstance, RunspacePool runspacePool)
+        public CommandInfoCache(Helper pssaHelperInstance)
         {
             _commandInfoCache = new ConcurrentDictionary<CommandLookupKey, Lazy<CommandInfo>>();
             _helperInstance = pssaHelperInstance;
-            _runspacePool = runspacePool;
+            _runspacePool = RunspaceFactory.CreateRunspacePool(1, 10);
+            _runspacePool.Open();
+        }
+
+        /// <summary>Dispose the runspace pool</summary>
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+
+        protected virtual void Dispose(bool disposing)
+        {
+            if ( disposed )
+            {
+                return;
+            }
+
+            if ( disposing )
+            {
+                _runspacePool.Dispose();
+            }
+
+            disposed = true;
         }
 
         /// <summary>

Engine/Helper.cs

@@ -30,7 +30,6 @@ public class Helper
         private PSVersionTable psVersionTable;
 
         private readonly Lazy<CommandInfoCache> _commandInfoCacheLazy;
-        private readonly RunspacePool _runSpacePool;
         private readonly object _testModuleManifestLock = new object();
 
         #endregion
@@ -116,11 +115,7 @@ internal set
         /// </summary>
         private Helper()
         {
-            // There are 5 rules that use the CommandInfo cache but one rule (AvoidAlias) makes parallel queries.
-            // Therefore 10 runspaces was a heuristic measure where no more speed improvement was seen.
-            _runSpacePool = RunspaceFactory.CreateRunspacePool(1, 10);
-            _runSpacePool.Open();
-            _commandInfoCacheLazy = new Lazy<CommandInfoCache>(() => new CommandInfoCache(pssaHelperInstance: this, runspacePool: _runSpacePool));
+            _commandInfoCacheLazy = new Lazy<CommandInfoCache>(() => new CommandInfoCache(pssaHelperInstance: this));
         }
 
         /// <summary>
@@ -309,7 +304,6 @@ public PSModuleInfo GetModuleManifest(string filePath, out IEnumerable<ErrorReco
             {
                 using (var ps = System.Management.Automation.PowerShell.Create())
                 {
-                    ps.RunspacePool = _runSpacePool;
                     ps.AddCommand("Test-ModuleManifest")
                       .AddParameter("Path", filePath)
                       .AddParameter("WarningAction", ActionPreference.SilentlyContinue);

Tests/Engine/Helper.tests.ps1

@@ -26,4 +26,11 @@ Describe "Test Directed Graph" {
             $digraph.IsConnected('v1', 'v4') | Should -BeTrue
         }
     }
+
+    Context "Runspaces should be disposed" {
+        It "Running analyzer 100 times should only create a limited number of runspaces" -Skip:$($PSVersionTable.PSVersion.Major -le 4) {
+            $null = 1..100 | ForEach-Object { Invoke-ScriptAnalyzer -ScriptDefinition 'gci' }
+            (Get-Runspace).Count | Should -BeLessOrEqual 10 -Because 'Number of Runspaces should not exceed size of runspace pool cache in CommandInfoCache'
+        }
+    }
 }