Skip to content

fix(http): prevent recursive session cleanup stack overflow #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
maxgolov merged 3 commits into from
May 2, 2026
Merged

fix(http): prevent recursive session cleanup stack overflow #52

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ca2782
fix(http): prevent recursive session cleanup stack overflow
maxgolov May 1, 2026
8354b98
Potential fix for pull request finding 'CodeQL / Prototype-polluting …
maxgolov May 1, 2026
a8b892f
fix: change transports and sessionTimeouts to Map<string, T> to fix T…
maxgolov May 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 34 additions & 20 deletions src/http-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ const refreshOnStart = process.env.ADVISORY_REFRESH_ON_START !== 'false'; // Def
const refreshIntervalMs = parseInt(process.env.ADVISORY_REFRESH_INTERVAL_MS || '0'); // Default: disabled (0)

// Store transports by session ID
const transports: { [sessionId: string]: StreamableHTTPServerTransport } = {};
const sessionTimeouts: Record<string, NodeJS.Timeout> = {};
const transports = new Map<string, StreamableHTTPServerTransport>();
const sessionTimeouts = new Map<string, NodeJS.Timeout>();
const cleanupInProgress = new Set<string>();
const SESSION_TIMEOUT_MS = 30 * 60 * 1000; // 30 minutes

// Periodic refresh cleanup function
Expand All @@ -60,33 +61,46 @@ let stopPeriodicRefresh: (() => void) | null = null;
* Clean up expired session
*/
function cleanupSession(sessionId: string): void {
if (cleanupInProgress.has(sessionId)) {
return;
}

cleanupInProgress.add(sessionId);
logger.info('Cleaning up session', { sessionId });
const transport = transports[sessionId];
if (transport) {
try {
transport.close();
} catch (error) {
logger.warn('Error closing transport', { sessionId, error: error instanceof Error ? error.message : String(error) });
try {
const transport = transports.get(sessionId);
if (transport) {
// Remove from registry first and detach onclose to avoid recursive cleanup.
transports.delete(sessionId);
transport.onclose = undefined;
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
try {
transport.close();
} catch (error) {
logger.warn('Error closing transport', { sessionId, error: error instanceof Error ? error.message : String(error) });
}
}
delete transports[sessionId];
}
if (sessionTimeouts[sessionId]) {
clearTimeout(sessionTimeouts[sessionId]);
delete sessionTimeouts[sessionId];
const timeout = sessionTimeouts.get(sessionId);
if (timeout) {
clearTimeout(timeout);
sessionTimeouts.delete(sessionId);
}
} finally {
cleanupInProgress.delete(sessionId);
}
}

/**
* Reset session timeout
*/
function resetSessionTimeout(sessionId: string): void {
if (sessionTimeouts[sessionId]) {
clearTimeout(sessionTimeouts[sessionId]);
const existingTimeout = sessionTimeouts.get(sessionId);
if (existingTimeout) {
clearTimeout(existingTimeout);
}
sessionTimeouts[sessionId] = setTimeout(() => {
sessionTimeouts.set(sessionId, setTimeout(() => {
logger.info('Session timeout', { sessionId });
cleanupSession(sessionId);
}, SESSION_TIMEOUT_MS);
}, SESSION_TIMEOUT_MS));
}

// Store local API server instance
Expand Down Expand Up @@ -154,9 +168,9 @@ app.post("/mcp", async (req: Request, res: Response) => {
try {
let transport: StreamableHTTPServerTransport;

if (sessionId && transports[sessionId]) {
if (sessionId && transports.has(sessionId)) {
// Reuse existing transport and reset timeout
transport = transports[sessionId];
transport = transports.get(sessionId)!;
resetSessionTimeout(sessionId);
await transport.handleRequest(req, res, req.body);
} else if (!sessionId && isInitializeRequest(req.body)) {
Expand All @@ -172,7 +186,7 @@ app.post("/mcp", async (req: Request, res: Response) => {
});

// Store transport
transports[newSessionId] = transport;
transports.set(newSessionId, transport);

// Set up session timeout
resetSessionTimeout(newSessionId);
Expand Down
Loading