Clean up the IPC named pipe on PowerShell exit (PowerShell#12187)

jitendriyag2 · Mar 26, 2020 · 668d72c · 668d72c
1 parent 238cb5c
commit 668d72c
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 29 deletions.
diff --git a/src/System.Management.Automation/engine/remoting/commands/EnterPSHostProcessCommand.cs b/src/System.Management.Automation/engine/remoting/commands/EnterPSHostProcessCommand.cs
@@ -707,7 +707,7 @@ internal static IReadOnlyCollection<PSHostProcessInfo> GetAppDomainNamesFromProc
                                     else if (process.ProcessName.Equals(pName, StringComparison.Ordinal))
                                     {
                                         // only add if the process name matches
-                                        procAppDomainInfo.Add(new PSHostProcessInfo(pName, id, appDomainName));
+                                        procAppDomainInfo.Add(new PSHostProcessInfo(pName, id, appDomainName, namedPipe));
                                     }
                                 }
                             }
@@ -736,6 +736,12 @@ internal static IReadOnlyCollection<PSHostProcessInfo> GetAppDomainNamesFromProc
     /// </summary>
     public sealed class PSHostProcessInfo
     {
+        #region Members
+
+        private readonly string _pipeNameFilePath;
+
+        #endregion
+
         #region Properties
 
         /// <summary>
@@ -781,29 +787,60 @@ public string MainWindowTitle
         private PSHostProcessInfo() { }
 
         /// <summary>
-        /// Constructor.
+        /// Initializes a new instance of the PSHostProcessInfo type.
         /// </summary>
         /// <param name="processName">Name of process.</param>
         /// <param name="processId">Id of process.</param>
         /// <param name="appDomainName">Name of process AppDomain.</param>
-        internal PSHostProcessInfo(string processName, int processId, string appDomainName)
+        /// <param name="pipeNameFilePath">File path of pipe name.</param>
+        internal PSHostProcessInfo(
+            string processName,
+            int processId,
+            string appDomainName,
+            string pipeNameFilePath)
         {
-            if (string.IsNullOrEmpty(processName)) { throw new PSArgumentNullException("processName"); }
+            if (string.IsNullOrEmpty(processName))
+            {
+                throw new PSArgumentNullException(nameof(processName));
+            }
 
-            if (string.IsNullOrEmpty(appDomainName)) { throw new PSArgumentNullException("appDomainName"); }
+            if (string.IsNullOrEmpty(appDomainName))
+            {
+                throw new PSArgumentNullException(nameof(appDomainName));
+            }
 
             MainWindowTitle = string.Empty;
             try
             {
                 var proc = Process.GetProcessById(processId);
                 MainWindowTitle = proc.MainWindowTitle ?? string.Empty;
             }
-            catch (ArgumentException) { }
-            catch (InvalidOperationException) { }
+            catch (ArgumentException)
+            {
+                // Window title is optional.
+            }
+            catch (InvalidOperationException)
+            {
+                // Window title is optional.
+            }
 
             this.ProcessName = processName;
             this.ProcessId = processId;
             this.AppDomainName = appDomainName;
+            _pipeNameFilePath = pipeNameFilePath;
+        }
+
+        #endregion
+
+        #region Methods
+
+        /// <summary>
+        /// Retrieves the pipe name file path.
+        /// </summary>
+        /// <returns>Pipe name file path.</returns>
+        public string GetPipeNameFilePath()
+        {
+            return _pipeNameFilePath;
         }
 
         #endregion

diff --git a/src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs b/src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs
@@ -560,9 +560,7 @@ static RemoteSessionNamedPipeServer()
 
             CreateIPCNamedPipeServerSingleton();
 
-#if !CORECLR // There is only one AppDomain per application in CoreCLR, which would be the default
-            CreateAppDomainUnloadHandler();
-#endif
+            CreateProcessExitHandler();
         }
 
         #endregion
@@ -961,30 +959,31 @@ internal static void CreateIPCNamedPipeServerSingleton()
             }
         }
 
-#if !CORECLR // There is only one AppDomain per application in CoreCLR, which would be the default
-        private static void CreateAppDomainUnloadHandler()
+        private static void CreateProcessExitHandler()
         {
-            // Subscribe to the app domain unload event.
-            AppDomain.CurrentDomain.DomainUnload += (sender, args) =>
+            AppDomain.CurrentDomain.ProcessExit += (sender, args) =>
+            {
+                IPCNamedPipeServerEnabled = false;
+                RemoteSessionNamedPipeServer namedPipeServer = IPCNamedPipeServer;
+                if (namedPipeServer != null)
                 {
-                    IPCNamedPipeServerEnabled = false;
-                    RemoteSessionNamedPipeServer namedPipeServer = IPCNamedPipeServer;
-                    if (namedPipeServer != null)
+                    try
                     {
-                        try
-                        {
-                            // Terminate the IPC thread.
-                            namedPipeServer.Dispose();
-                        }
-                        catch (ObjectDisposedException) { }
-                        catch (Exception)
-                        {
-                            // Don't throw an exception on the app domain unload event thread.
-                        }
+                        // Terminate the IPC thread.
+                        namedPipeServer.Dispose();
                     }
-                };
+                    catch (ObjectDisposedException)
+                    {
+                        // Ignore if object already disposed.
+                    }
+                    catch (Exception)
+                    {
+                        // Don't throw an exception on the app domain unload event thread.
+                    }
+                }
+            };
         }
-#endif
+
         private static void OnIPCNamedPipeServerEnded(object sender, ListenerEndedEventArgs args)
         {
             if (args.RestartListener)

diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-PSHostProcessInfo.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-PSHostProcessInfo.Tests.ps1
@@ -53,4 +53,49 @@ Describe "Get-PSHostProcessInfo tests" -Tag CI {
         $psProcess.Count | Should -BeGreaterOrEqual 1
         $psProcess.ProcessId | Should -Contain $powershell.id
     }
+
+    It "Verifies named pipe filepath get method" {
+        $pipeFilePath = (Get-PSHostProcessInfo -Id $pid).GetPipeNameFilePath()
+        $pipeFilePath | Should -Exist
+    }
+
+    It "Verifies named pipe filepath is removed on process exit" {
+        $aliveFile = Join-Path -Path $TestDrive -ChildPath 'AliveFileXXZZ.txt'
+        "" | Out-File -FilePath $aliveFile
+        $testfilePath = Join-Path -Path $TestDrive -ChildPath 'TestScriptXXZZ.ps1'
+        @'
+            param (
+                [string] $LiveFilePath
+            )
+
+            $count = 0
+            while ((Test-Path -Path $LiveFilePath) -and ($count++ -lt 60))
+            {
+                Start-Sleep -Milliseconds 500
+            }
+
+            exit
+'@ | Out-File -FilePath $testfilePath
+
+        # Create PowerShell process to monitor.
+        $psFileName = $IsWindows ? 'pwsh.exe' : 'pwsh'
+        $psPath = Join-Path -Path $PSHOME -ChildPath $psFileName
+        $psProc = Start-Process -FilePath $psPath -ArgumentList "-File $testfilePath -LiveFilePath $aliveFile" -PassThru
+        Wait-UntilTrue -sb {
+            (Get-PSHostProcessInfo -Id $psProc.Id) -ne $null
+        } -TimeoutInMilliseconds 5000 -IntervalInMilliseconds 250
+
+        # Verify named pipe file path.
+        $psNamedPipePath = (Get-PSHostProcessInfo -Id $psProc.Id).GetPipeNameFilePath()
+        $psNamedPipePath | Should -Exist
+
+        # Signal PowerShell test process to exit normally.
+        Remove-Item -Path $aliveFile -Force -ErrorAction Ignore
+        Wait-UntilTrue -sb {
+            (Test-Path -Path $psNamedPipePath) -eq $false
+        } -TimeoutInMilliseconds 5000 -IntervalInMilliseconds 250
+
+        # Verify named pipe file path is removed.
+        $psNamedPipePath | Should -Not -Exist
+    }
 }