From 7a71c3c3f89e7759f36fbc8a79e4b868d3f6a49c Mon Sep 17 00:00:00 2001 From: czlonkowski <56956555+czlonkowski@users.noreply.github.com> Date: Sat, 2 Aug 2025 14:45:58 +0200 Subject: [PATCH 1/2] fix: memory leak in SimpleCache causing MCP connection loss (fixes #118) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- docs/CHANGELOG.md | 8 ++ src/http-server-single-session.ts | 26 +++- src/mcp/server.ts | 10 ++ src/utils/simple-cache.ts | 15 ++- .../simple-cache-memory-leak-fix.test.ts | 123 ++++++++++++++++++ 5 files changed, 178 insertions(+), 4 deletions(-) create mode 100644 tests/unit/utils/simple-cache-memory-leak-fix.test.ts diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 4db91b6..eb6095a 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **Memory Leak in SimpleCache**: Fixed critical memory leak causing MCP server connection loss after several hours (fixes #118) + - Added proper timer cleanup in `SimpleCache.destroy()` method + - Updated MCP server shutdown to clean up cache timers + - Enhanced HTTP server error handling with transport error handlers + - Fixed event listener cleanup to prevent accumulation + - Added comprehensive test coverage for memory leak prevention + ## [2.10.0] - 2025-08-02 ### Added diff --git a/src/http-server-single-session.ts b/src/http-server-single-session.ts index 59e1566..6c8e771 100644 --- a/src/http-server-single-session.ts +++ b/src/http-server-single-session.ts @@ -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); }); } diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 472c954..b081592 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -2538,6 +2538,16 @@ Full documentation is being prepared. For now, use get_node_essentials for confi async shutdown(): Promise { 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 { diff --git a/src/utils/simple-cache.ts b/src/utils/simple-cache.ts index c2d8088..cbbad82 100644 --- a/src/utils/simple-cache.ts +++ b/src/utils/simple-cache.ts @@ -4,10 +4,11 @@ */ export class SimpleCache { private cache = new Map(); + 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(); + } } \ No newline at end of file diff --git a/tests/unit/utils/simple-cache-memory-leak-fix.test.ts b/tests/unit/utils/simple-cache-memory-leak-fix.test.ts new file mode 100644 index 0000000..eea96cf --- /dev/null +++ b/tests/unit/utils/simple-cache-memory-leak-fix.test.ts @@ -0,0 +1,123 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { SimpleCache } from '../../../src/utils/simple-cache'; + +describe('SimpleCache Memory Leak Fix', () => { + let cache: SimpleCache; + + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + if (cache && typeof cache.destroy === 'function') { + cache.destroy(); + } + vi.restoreAllMocks(); + }); + + it('should track cleanup timer', () => { + cache = new SimpleCache(); + // Access private property for testing + expect((cache as any).cleanupTimer).toBeDefined(); + expect((cache as any).cleanupTimer).not.toBeNull(); + }); + + it('should clear timer on destroy', () => { + cache = new SimpleCache(); + const timer = (cache as any).cleanupTimer; + + cache.destroy(); + + expect((cache as any).cleanupTimer).toBeNull(); + // Verify timer was cleared + expect(() => clearInterval(timer)).not.toThrow(); + }); + + it('should clear cache on destroy', () => { + cache = new SimpleCache(); + cache.set('test-key', 'test-value', 300); + + expect(cache.get('test-key')).toBe('test-value'); + + cache.destroy(); + + expect(cache.get('test-key')).toBeNull(); + }); + + it('should handle multiple destroy calls safely', () => { + cache = new SimpleCache(); + + expect(() => { + cache.destroy(); + cache.destroy(); + cache.destroy(); + }).not.toThrow(); + + expect((cache as any).cleanupTimer).toBeNull(); + }); + + it('should not create new timers after destroy', () => { + cache = new SimpleCache(); + const originalTimer = (cache as any).cleanupTimer; + + cache.destroy(); + + // Try to use the cache after destroy + cache.set('key', 'value'); + cache.get('key'); + cache.clear(); + + // Timer should still be null + expect((cache as any).cleanupTimer).toBeNull(); + expect((cache as any).cleanupTimer).not.toBe(originalTimer); + }); + + it('should clean up expired entries periodically', () => { + cache = new SimpleCache(); + + // Set items with different TTLs + cache.set('short', 'value1', 1); // 1 second + cache.set('long', 'value2', 300); // 300 seconds + + // Advance time by 2 seconds + vi.advanceTimersByTime(2000); + + // Advance time to trigger cleanup (60 seconds) + vi.advanceTimersByTime(58000); + + // Short-lived item should be gone + expect(cache.get('short')).toBeNull(); + // Long-lived item should still exist + expect(cache.get('long')).toBe('value2'); + }); + + it('should prevent memory leak by clearing timer', () => { + const timers: NodeJS.Timeout[] = []; + const originalSetInterval = global.setInterval; + + // Mock setInterval to track created timers + global.setInterval = vi.fn((callback, delay) => { + const timer = originalSetInterval(callback, delay); + timers.push(timer); + return timer; + }); + + // Create and destroy multiple caches + for (let i = 0; i < 5; i++) { + const tempCache = new SimpleCache(); + tempCache.set(`key${i}`, `value${i}`); + tempCache.destroy(); + } + + // All timers should have been cleared + expect(timers.length).toBe(5); + + // Restore original setInterval + global.setInterval = originalSetInterval; + }); + + it('should have destroy method defined', () => { + cache = new SimpleCache(); + expect(typeof cache.destroy).toBe('function'); + }); +}); \ No newline at end of file From df4066022f94584c8edd59cb829ecb9a0a848e5d Mon Sep 17 00:00:00 2001 From: czlonkowski <56956555+czlonkowski@users.noreply.github.com> Date: Sat, 2 Aug 2025 15:18:58 +0200 Subject: [PATCH 2/2] chore: bump version to 2.10.1 for memory leak fix release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Updated version in package.json and package.runtime.json - Updated README version badge - Moved changelog entry from Unreleased to v2.10.1 - Added version comparison link for v2.10.1 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- README.md | 2 +- docs/CHANGELOG.md | 3 +++ package.json | 2 +- package.runtime.json | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 2c6fe49..2e80366 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) [![GitHub stars](https://img.shields.io/github/stars/czlonkowski/n8n-mcp?style=social)](https://github.com/czlonkowski/n8n-mcp) -[![Version](https://img.shields.io/badge/version-2.10.0-blue.svg)](https://github.com/czlonkowski/n8n-mcp) +[![Version](https://img.shields.io/badge/version-2.10.1-blue.svg)](https://github.com/czlonkowski/n8n-mcp) [![npm version](https://img.shields.io/npm/v/n8n-mcp.svg)](https://www.npmjs.com/package/n8n-mcp) [![codecov](https://codecov.io/gh/czlonkowski/n8n-mcp/graph/badge.svg?token=YOUR_TOKEN)](https://codecov.io/gh/czlonkowski/n8n-mcp) [![Tests](https://img.shields.io/badge/tests-1356%20passing-brightgreen.svg)](https://github.com/czlonkowski/n8n-mcp/actions) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index eb6095a..c7870d5 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.10.1] - 2025-08-02 + ### Fixed - **Memory Leak in SimpleCache**: Fixed critical memory leak causing MCP server connection loss after several hours (fixes #118) - Added proper timer cleanup in `SimpleCache.destroy()` method @@ -1082,6 +1084,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Basic n8n and MCP integration - Core workflow automation features +[2.10.1]: https://github.com/czlonkowski/n8n-mcp/compare/v2.10.0...v2.10.1 [2.10.0]: https://github.com/czlonkowski/n8n-mcp/compare/v2.9.1...v2.10.0 [2.9.1]: https://github.com/czlonkowski/n8n-mcp/compare/v2.9.0...v2.9.1 [2.9.0]: https://github.com/czlonkowski/n8n-mcp/compare/v2.8.3...v2.9.0 diff --git a/package.json b/package.json index ca702ad..3d04d0a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "n8n-mcp", - "version": "2.10.0", + "version": "2.10.1", "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)", "main": "dist/index.js", "bin": { diff --git a/package.runtime.json b/package.runtime.json index 6155d19..90a0d77 100644 --- a/package.runtime.json +++ b/package.runtime.json @@ -1,6 +1,6 @@ { "name": "n8n-mcp-runtime", - "version": "2.10.0", + "version": "2.10.1", "description": "n8n MCP Server Runtime Dependencies Only", "private": true, "dependencies": {