Skip to content

Conversation

nick-pape
Copy link

In STDIO mode, the server-everything server has a lifecycle issue that causes non-standard behavior. When a client calls close(), the timeouts are not cleaned up and so the process never exits.

Description

Server Details

@modelcontextprotocol/server-everything, fix exit/cleanup lifecycle.

Motivation and Context

You can repro this with a minimal client:

const { Client } = require('@modelcontextprotocol/sdk/client');
const { StdioClientTransport } = require('@modelcontextprotocol/sdk/client/stdio.js');

const client = new Client({
    name: 'Test Client',
    version: '0.0.1',
    capabilities: {}
});

const transport = new StdioClientTransport({
    name: "proxy:server-everything",
    type: "stdio",
    command: "npx",
    args: ["-y", "@modelcontextprotocol/server-everything"]
});

client.connect(transport).then(() => {
    console.log("Client connected");

    // Does not work (results in hanging)
    return client.close().then(() => {
        console.log("Client closed");
    });
});

You'll see the client is closed, but the process remains running because the child process's interval handlers have not been removed (cleanup()).

I tried various things to fix this, e.g. sending a SIGINT/SIGTERM/SIGKILL to the child. However, the PID you see in the transport is the NPX process, not it's child (the MCP server).

E.g. launching via NPX neither this, nor a using SIGKILL, works (the pipes remain open and your process never exits):

client.connect(transport).then(() => {
    console.log("Client connected");

    console.log("Sending SIGINT");
    process.kill(transport.pid, 'SIGINT');

    // Does not work (results in hanging)
    // even if this block is in a timeout, using SIGKILL, etc
    return client.close().then(() => {
        console.log("Client closed");
    });
});

(Notable the above code does work if the transport is connecting to node everything/index.js).

This is non-standard for STDIO servers, I think. E.g. the first example works with a litany of other servers (including other examples in this repo like sequential-thinking). Presumably in STDIO mode, the parent process is the client (or at least proxying pipes to client). In any case, the client calling close() should result in termination of the everything MCP.

How Has This Been Tested?

I tested this using the first minimal example above. The first example worked properly with local (non-NPX) invocation of server-everything in my branch, (whereas it did not work in main).

I also verified that (when doing a local node invocation) the behavior of SIGINT remained the same.

Breaking Changes

No breaking changes.

Types of changes

  • 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

  • I have read the MCP Protocol Documentation
  • My changes follows MCP security best practices
  • I have updated the server's README accordingly
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have documented all environment variables and configuration options

Additional context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant