Skip to content

Conversation

@tianzhou
Copy link
Member

Create server once and reuse for all requests

Copilot AI review requested due to automatic review settings December 26, 2025 06:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes the MCP server architecture by creating the server instance once at startup and reusing it across all HTTP requests, rather than creating a new instance per request. This improves performance by eliminating redundant tool registration overhead.

Key changes:

  • Server instance is now created once and shared across all requests to avoid repeated tool registration
  • Transport instances are still created per request to maintain isolation
  • Removed redundant per-tool logging since registration now only happens once at startup

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/server.ts Moved server instantiation outside request handler to enable reuse; transport creation remains per-request for isolation
src/tools/index.ts Removed per-tool console logging as tools are now only registered once at startup
Comments suppressed due to low confidence (1)

src/server.ts:223

  • The change to reuse a single server instance across all requests should include test coverage for concurrent request handling. Consider adding a test that:
  1. Sends multiple simultaneous requests to the /mcp endpoint
  2. Verifies all requests complete successfully without errors
  3. Confirms no request ID collisions occur
  4. Checks that responses are correctly matched to their respective requests

This would validate that the MCP server safely handles concurrent connections to the same server instance.

      app.post("/mcp", async (req, res) => {
        try {
          // Create a new transport for each request to ensure isolation
          // The server instance (with registered tools) is reused
          const transport = new StreamableHTTPServerTransport({
            sessionIdGenerator: undefined, // Disable session management for stateless mode
            enableJsonResponse: true // Use JSON responses (SSE not supported in stateless mode)
          });

          await server.connect(transport);
          await transport.handleRequest(req, res, req.body);
        } catch (error) {
          console.error("Error handling request:", error);
          if (!res.headersSent) {
            res.status(500).json({ error: 'Internal server error' });
          }
        }
      });

src/server.ts Outdated
Comment on lines 204 to 215
const server = createServer();

app.post("/mcp", async (req, res) => {
try {
// In stateless mode, create a new instance of transport and server for each request
// to ensure complete isolation. A single instance would cause request ID collisions
// when multiple clients connect concurrently.
// Create a new transport for each request to ensure isolation
// The server instance (with registered tools) is reused
const transport = new StreamableHTTPServerTransport({
sessionIdGenerator: undefined, // Disable session management for stateless mode
enableJsonResponse: true // Use JSON responses (SSE not supported in stateless mode)
});
const server = createServer();

await server.connect(transport);
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reusing a single server instance across concurrent requests may cause request ID collisions and state corruption. The original implementation created a new server instance per request specifically to avoid this issue in stateless mode.

The current code calls server.connect(transport) for each incoming request without any corresponding disconnect or cleanup. This could lead to:

  1. Multiple concurrent connections to the same server instance causing request ID conflicts
  2. Potential memory leaks as transports accumulate without cleanup
  3. Race conditions when handling concurrent requests

Consider either:

  • Reverting to the original approach of creating a new server instance per request
  • Adding proper cleanup after each request (e.g., disconnecting the transport)
  • Verifying through load testing that the MCP SDK safely handles concurrent connect() calls on the same server instance

Copilot uses AI. Check for mistakes.
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.

2 participants