ros/n8n-mcp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #120 from czlonkowski/fix/issue-118-mcp-connection-loss

Browse Source 
### 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
This commit is contained in:
Romuald Członkowski
2025-08-02 15:24:53 +02:00
committed by GitHub
parent 3bfad51519 df4066022f
commit 9e71c71698
8 changed files with 184 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
README.md
View File

@@ -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)

11
docs/CHANGELOG.md
View File

@@ -7,6 +7,16 @@ 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
- 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
@@ -1074,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

2
package.json
View File

@@ -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": {

2
package.runtime.json
View File

@@ -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": {

26
src/http-server-single-session.ts
View File

@@ -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);
});
}

10
src/mcp/server.ts
View File

@@ -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 {

15
src/utils/simple-cache.ts
View File

@@ -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();
}
}

123
tests/unit/utils/simple-cache-memory-leak-fix.test.ts Normal file
View File

@@ -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');
});
});