fix: memory leak in SimpleCache causing MCP connection loss (fixes #118)
- Added cleanupTimer property to track setInterval timer - Implemented destroy() method to clear timer and prevent memory leak - Updated MCP server shutdown to call cache.destroy() - Enhanced HTTP server error handling with transport.onerror - Fixed event listener cleanup to prevent accumulation - Added comprehensive test coverage for memory leak prevention This fixes the issue where MCP server would lose connection after several hours due to timer accumulation causing memory exhaustion. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -369,7 +369,7 @@ export class SingleSessionHTTPServer {
|
||||
}
|
||||
});
|
||||
|
||||
// Set up cleanup handler
|
||||
// Set up cleanup handlers
|
||||
transport.onclose = () => {
|
||||
const sid = transport.sessionId;
|
||||
if (sid) {
|
||||
@@ -378,6 +378,17 @@ export class SingleSessionHTTPServer {
|
||||
}
|
||||
};
|
||||
|
||||
// Handle transport errors to prevent connection drops
|
||||
transport.onerror = (error: Error) => {
|
||||
const sid = transport.sessionId;
|
||||
logger.error('Transport error', { sessionId: sid, error: error.message });
|
||||
if (sid) {
|
||||
this.removeSession(sid, 'transport_error').catch(err => {
|
||||
logger.error('Error during transport error cleanup', { error: err });
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
// Connect the server to the transport BEFORE handling the request
|
||||
logger.info('handleRequest: Connecting server to new transport');
|
||||
await server.connect(transport);
|
||||
@@ -873,7 +884,7 @@ export class SingleSessionHTTPServer {
|
||||
const sessionId = req.headers['mcp-session-id'] as string | undefined;
|
||||
// Only add event listener if the request object supports it (not in test mocks)
|
||||
if (typeof req.on === 'function') {
|
||||
req.on('close', () => {
|
||||
const closeHandler = () => {
|
||||
if (!res.headersSent && sessionId) {
|
||||
logger.info('Connection closed before response sent', { sessionId });
|
||||
// Schedule immediate cleanup if connection closes unexpectedly
|
||||
@@ -883,11 +894,20 @@ export class SingleSessionHTTPServer {
|
||||
const timeSinceAccess = Date.now() - metadata.lastAccess.getTime();
|
||||
// Only remove if it's been inactive for a bit to avoid race conditions
|
||||
if (timeSinceAccess > 60000) { // 1 minute
|
||||
this.removeSession(sessionId, 'connection_closed');
|
||||
this.removeSession(sessionId, 'connection_closed').catch(err => {
|
||||
logger.error('Error during connection close cleanup', { error: err });
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
req.on('close', closeHandler);
|
||||
|
||||
// Clean up event listener when response ends to prevent memory leaks
|
||||
res.on('finish', () => {
|
||||
req.removeListener('close', closeHandler);
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -2538,6 +2538,16 @@ Full documentation is being prepared. For now, use get_node_essentials for confi
|
||||
async shutdown(): Promise<void> {
|
||||
logger.info('Shutting down MCP server...');
|
||||
|
||||
// Clean up cache timers to prevent memory leaks
|
||||
if (this.cache) {
|
||||
try {
|
||||
this.cache.destroy();
|
||||
logger.info('Cache timers cleaned up');
|
||||
} catch (error) {
|
||||
logger.error('Error cleaning up cache:', error);
|
||||
}
|
||||
}
|
||||
|
||||
// Close database connection if it exists
|
||||
if (this.db) {
|
||||
try {
|
||||
|
||||
@@ -4,10 +4,11 @@
|
||||
*/
|
||||
export class SimpleCache {
|
||||
private cache = new Map<string, { data: any; expires: number }>();
|
||||
private cleanupTimer: NodeJS.Timeout | null = null;
|
||||
|
||||
constructor() {
|
||||
// Clean up expired entries every minute
|
||||
setInterval(() => {
|
||||
this.cleanupTimer = setInterval(() => {
|
||||
const now = Date.now();
|
||||
for (const [key, item] of this.cache.entries()) {
|
||||
if (item.expires < now) this.cache.delete(key);
|
||||
@@ -34,4 +35,16 @@ export class SimpleCache {
|
||||
clear(): void {
|
||||
this.cache.clear();
|
||||
}
|
||||
|
||||
/**
|
||||
* Clean up the cache and stop the cleanup timer
|
||||
* Essential for preventing memory leaks in long-running servers
|
||||
*/
|
||||
destroy(): void {
|
||||
if (this.cleanupTimer) {
|
||||
clearInterval(this.cleanupTimer);
|
||||
this.cleanupTimer = null;
|
||||
}
|
||||
this.cache.clear();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user