Conversation
Check if parent process is still running otherwise exit. Due to not to hanging ps in Processes each time PSMCP is used by LLM
There was a problem hiding this comment.
Pull request overview
This PR adds a parent process monitoring mechanism to prevent ghost PowerShell processes by checking if the parent process is still running and exiting if it's not. The goal is to avoid hanging ps processes each time the PSMCP is used by an LLM.
Key Changes:
- Added parent process health check in the main server loop
- Implemented graceful shutdown when parent process exits
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Check if parent process is still running otherwise exit | ||
| try { | ||
| $parentProcess = Get-Process -Id $parentProcessId -ErrorAction Stop |
There was a problem hiding this comment.
The variable $parentProcessId is not defined anywhere in the function. It's not included in the function parameters, and there's no evidence of it being set in the module scope. This code will fail at runtime because PowerShell will not be able to resolve this variable. Consider adding $parentProcessId as a parameter to the function, or use the automatic variable $PID with a mechanism to track the parent process ID.
There was a problem hiding this comment.
Oops :) Sorry, seems I forgot to insert from the beginning 🤦♂️ Will fix it now
# Get parent process ID to monitor for disconnection
$currentProcess = Get-Process -Id $PID
$parentProcessId = $currentProcess.Parent.Id
Write-Log -LogEntry @{ Level = 'Info'; Message = "MCP Server started with PID: $PID, Parent PID: $parentProcessId" }| if ($null -eq $parentProcess) { | ||
| Write-Log -LogEntry @{ Level = 'Info'; Message = "Parent process has exited. Shutting down MCP server..." } | ||
| exit | ||
| } |
There was a problem hiding this comment.
This null check is unreachable code. When Get-Process -ErrorAction Stop succeeds, it will always return a process object, never null. If the process doesn't exist, it throws an exception which is caught by the catch block. This condition at line 20-23 will never execute. The catch block already handles the case when the parent process is not found.
| if ($null -eq $parentProcess) { | |
| Write-Log -LogEntry @{ Level = 'Info'; Message = "Parent process has exited. Shutting down MCP server..." } | |
| exit | |
| } |
| while ($true) { | ||
|
|
||
| # Check if parent process is still running otherwise exit | ||
| try { | ||
| $parentProcess = Get-Process -Id $parentProcessId -ErrorAction Stop | ||
| if ($null -eq $parentProcess) { | ||
| Write-Log -LogEntry @{ Level = 'Info'; Message = "Parent process has exited. Shutting down MCP server..." } | ||
| exit | ||
| } | ||
| } | ||
| catch { | ||
| Write-Log -LogEntry @{ Level = 'Info'; Message = "Parent process not found. Shutting down MCP server..." } | ||
| exit | ||
| } |
There was a problem hiding this comment.
Checking if the parent process is running on every iteration of the while loop could impact performance, especially when processing many requests. Consider checking the parent process less frequently, such as every N iterations or using a timer-based approach, to reduce the overhead while still catching orphaned processes in a reasonable timeframe.
| while ($true) { | |
| # Check if parent process is still running otherwise exit | |
| try { | |
| $parentProcess = Get-Process -Id $parentProcessId -ErrorAction Stop | |
| if ($null -eq $parentProcess) { | |
| Write-Log -LogEntry @{ Level = 'Info'; Message = "Parent process has exited. Shutting down MCP server..." } | |
| exit | |
| } | |
| } | |
| catch { | |
| Write-Log -LogEntry @{ Level = 'Info'; Message = "Parent process not found. Shutting down MCP server..." } | |
| exit | |
| } | |
| # Control how often we check whether the parent process is still running | |
| $parentCheckInterval = 100 | |
| $iterationCount = 0 | |
| while ($true) { | |
| $iterationCount++ | |
| # Check if parent process is still running otherwise exit (only every N iterations) | |
| if ($iterationCount % $parentCheckInterval -eq 0) { | |
| try { | |
| $parentProcess = Get-Process -Id $parentProcessId -ErrorAction Stop | |
| if ($null -eq $parentProcess) { | |
| Write-Log -LogEntry @{ Level = 'Info'; Message = "Parent process has exited. Shutting down MCP server..." } | |
| exit | |
| } | |
| } | |
| catch { | |
| Write-Log -LogEntry @{ Level = 'Info'; Message = "Parent process not found. Shutting down MCP server..." } | |
| exit | |
| } | |
| } |
added forgotten $parentProcessId
|
@dfinke seems only you are able to run Copilot review, Please rerun it once more |
|
BTW Seems it works only in PowerShell 7+ 🤔 $currentProcess = Get-Process -Id $PID
$parentProcessId = $currentProcess.Parent.Id |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Cool. Copilot in motion. Yeah only the repo owner can kick it off. I asked AI about that snippet. 
Good stuff |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Well, since you are using |
Check if parent process is still running otherwise exit. Due to not to hanging ps in Processes each time PSMCP is used by LLM