Fix child process kill error with npx based servers

[surya-prakash-susarla]

Motivation and Context

When running npx based servers in stdio mode, the npx process spawns a sub-process. During exit, the main process does not propagate the SIGTERM to the child process which leads to a failed kill signal and the stdin & stdout of the root process getting mangled.

The current change creates a new process group when launching the fork for the server and ensures that a kill signal is sent for the process group is the process does not shutdown cleanly. This would ensure that the process group (which includes all the child process) would be killed successfully to avoid any potential issues.

How Has This Been Tested?

Yes, a simple script like this:

from fastmcp import Client
import asyncio

async def test():
    config = {
        "mcpServers": {
            "everything": {
                "command": "npx",
                "args": ["-y", "@modelcontextprotocol/server-everything"]
            }
        }
    }

    async with Client(config) as client:
      tools = await client.list_tools()
      print("TOOlS: {tools}".format(tools=tools))

asyncio.run(test())

The above script will currently block the stdio of the parent server when run using as: python test.py
After the change the npx root & the node child process die succesfully allowing the control to flow back to the parent process ensuring a clean shutdown.

Breaking Changes

No, this should not be a breaking change since the process group flag is ignored in non-POSIX systems, and the cleanup via process group functionality is only applied for POSIX style launches.

Types of changes

• [ x ] Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• [ x ] I have read the MCP Documentation
• [ x ] My code follows the repository's style guidelines
• [ x ] New and existing tests pass locally
• [ x ] I have added appropriate error handling
• [ x ] I have added or updated documentation as needed

Additional context

Please refer to the issue here (slightly different context):
#547

[surya-prakash-susarla]

Failing test seems to be a streamable http test which was not modified in this change. Please let me know in case it's a side-effect of this change.

[felixweinberger]

Thank you @surya-prakash-susarla for submitting this PR and the detailed information on reproduction of the issue!

I spent yesterday and today trying to unify all the different approaches and fixes we have pending in this process termination space at the moment, as there are several interrelated fixes that either conflict or depend on each other - specifically #555, #729, #765, and #850.

I've added your change to #1044 as a draft with you as a co-author + added extensive regression testing. Would you be OK with consolidating this change into #1044 for the comprehensive testing & process handling introduced there? Would love your feedback on the PR as well if you have time!

[felixweinberger]

Requesting changes for now to put this PR in your queue - please take a look at my comment above regarding #1044 🙂