From bed1a76537866670be09bb2410e620690e60a14f Mon Sep 17 00:00:00 2001 From: Philipp Kunz Date: Sun, 1 Jun 2025 08:02:32 +0000 Subject: [PATCH] refactor(socket-utils): replace direct socket cleanup with centralized cleanupSocket utility across connection management --- ts/proxies/http-proxy/connection-pool.ts | 19 ++--------- ts/proxies/http-proxy/http-proxy.ts | 7 ++-- ts/proxies/smart-proxy/connection-manager.ts | 34 ++++--------------- ts/proxies/smart-proxy/port-manager.ts | 4 +-- .../smart-proxy/route-connection-handler.ts | 13 ++++--- 5 files changed, 19 insertions(+), 58 deletions(-) diff --git a/ts/proxies/http-proxy/connection-pool.ts b/ts/proxies/http-proxy/connection-pool.ts index ea018f7..a790fd3 100644 --- a/ts/proxies/http-proxy/connection-pool.ts +++ b/ts/proxies/http-proxy/connection-pool.ts @@ -1,5 +1,6 @@ import * as plugins from '../../plugins.js'; import { type IHttpProxyOptions, type IConnectionEntry, type ILogger, createLogger } from './models/types.js'; +import { cleanupSocket } from '../../core/utils/socket-utils.js'; /** * Manages a pool of backend connections for efficient reuse @@ -133,14 +134,7 @@ export class ConnectionPool { if ((connection.isIdle && now - connection.lastUsed > idleTimeout) || connections.length > (this.options.connectionPoolSize || 50)) { - try { - if (!connection.socket.destroyed) { - connection.socket.end(); - connection.socket.destroy(); - } - } catch (err) { - this.logger.error(`Error destroying pooled connection to ${host}`, err); - } + cleanupSocket(connection.socket, `pool-${host}-idle`); connections.shift(); // Remove from pool removed++; @@ -170,14 +164,7 @@ export class ConnectionPool { this.logger.debug(`Closing ${connections.length} connections to ${host}`); for (const connection of connections) { - try { - if (!connection.socket.destroyed) { - connection.socket.end(); - connection.socket.destroy(); - } - } catch (error) { - this.logger.error(`Error closing connection to ${host}:`, error); - } + cleanupSocket(connection.socket, `pool-${host}-close`); } } diff --git a/ts/proxies/http-proxy/http-proxy.ts b/ts/proxies/http-proxy/http-proxy.ts index 88c240a..986a02b 100644 --- a/ts/proxies/http-proxy/http-proxy.ts +++ b/ts/proxies/http-proxy/http-proxy.ts @@ -18,6 +18,7 @@ import { RequestHandler, type IMetricsTracker } from './request-handler.js'; import { WebSocketHandler } from './websocket-handler.js'; import { ProxyRouter } from '../../routing/router/index.js'; import { RouteRouter } from '../../routing/router/route-router.js'; +import { cleanupSocket } from '../../core/utils/socket-utils.js'; import { FunctionCache } from './function-cache.js'; /** @@ -520,11 +521,7 @@ export class HttpProxy implements IMetricsTracker { // Close all tracked sockets for (const socket of this.socketMap.getArray()) { - try { - socket.destroy(); - } catch (error) { - this.logger.error('Error destroying socket', error); - } + cleanupSocket(socket, 'http-proxy-stop'); } // Close all connection pool connections diff --git a/ts/proxies/smart-proxy/connection-manager.ts b/ts/proxies/smart-proxy/connection-manager.ts index 0acadba..6f911b7 100644 --- a/ts/proxies/smart-proxy/connection-manager.ts +++ b/ts/proxies/smart-proxy/connection-manager.ts @@ -4,6 +4,7 @@ import { SecurityManager } from './security-manager.js'; import { TimeoutManager } from './timeout-manager.js'; import { logger } from '../../core/utils/logger.js'; import { LifecycleComponent } from '../../core/utils/lifecycle-component.js'; +import { cleanupSocket } from '../../core/utils/socket-utils.js'; /** * Manages connection lifecycle, tracking, and cleanup with performance optimizations @@ -278,10 +279,10 @@ export class ConnectionManager extends LifecycleComponent { } // Handle socket cleanup without delay - this.cleanupSocketImmediate(record, 'incoming', record.incoming); + cleanupSocket(record.incoming, `${record.id}-incoming`); if (record.outgoing) { - this.cleanupSocketImmediate(record, 'outgoing', record.outgoing); + cleanupSocket(record.outgoing, `${record.id}-outgoing`); } // Clear pendingData to avoid memory leaks @@ -313,23 +314,6 @@ export class ConnectionManager extends LifecycleComponent { } } - /** - * Helper method to clean up a socket immediately - */ - private cleanupSocketImmediate(record: IConnectionRecord, side: 'incoming' | 'outgoing', socket: plugins.net.Socket): void { - try { - if (!socket.destroyed) { - socket.destroy(); - } - } catch (err) { - logger.log('error', `Error destroying ${side} socket: ${err}`, { - connectionId: record.id, - side, - error: err, - component: 'connection-manager' - }); - } - } /** * Creates a generic error handler for incoming or outgoing sockets @@ -552,19 +536,13 @@ export class ConnectionManager extends LifecycleComponent { record.cleanupTimer = undefined; } - // Immediate destruction + // Immediate destruction using socket-utils if (record.incoming) { - record.incoming.removeAllListeners(); - if (!record.incoming.destroyed) { - record.incoming.destroy(); - } + cleanupSocket(record.incoming, `${record.id}-incoming-shutdown`); } if (record.outgoing) { - record.outgoing.removeAllListeners(); - if (!record.outgoing.destroyed) { - record.outgoing.destroy(); - } + cleanupSocket(record.outgoing, `${record.id}-outgoing-shutdown`); } } catch (err) { logger.log('error', `Error during connection cleanup: ${err}`, { diff --git a/ts/proxies/smart-proxy/port-manager.ts b/ts/proxies/smart-proxy/port-manager.ts index e59c41f..b1fb6a3 100644 --- a/ts/proxies/smart-proxy/port-manager.ts +++ b/ts/proxies/smart-proxy/port-manager.ts @@ -2,6 +2,7 @@ import * as plugins from '../../plugins.js'; import type { ISmartProxyOptions } from './models/interfaces.js'; import { RouteConnectionHandler } from './route-connection-handler.js'; import { logger } from '../../core/utils/logger.js'; +import { cleanupSocket } from '../../core/utils/socket-utils.js'; /** * PortManager handles the dynamic creation and removal of port listeners @@ -64,8 +65,7 @@ export class PortManager { const server = plugins.net.createServer((socket) => { // Check if shutting down if (this.isShuttingDown) { - socket.end(); - socket.destroy(); + cleanupSocket(socket, 'port-manager-shutdown'); return; } diff --git a/ts/proxies/smart-proxy/route-connection-handler.ts b/ts/proxies/smart-proxy/route-connection-handler.ts index ed348f7..86a124a 100644 --- a/ts/proxies/smart-proxy/route-connection-handler.ts +++ b/ts/proxies/smart-proxy/route-connection-handler.ts @@ -9,7 +9,7 @@ import { TlsManager } from './tls-manager.js'; import { HttpProxyBridge } from './http-proxy-bridge.js'; import { TimeoutManager } from './timeout-manager.js'; import { RouteManager } from './route-manager.js'; -import type { ForwardingHandler } from '../../forwarding/handlers/base-handler.js'; +import { cleanupSocket } from '../../core/utils/socket-utils.js'; /** * Handles new connection processing and setup logic with support for route-based configuration @@ -84,8 +84,7 @@ export class RouteConnectionHandler { const ipValidation = this.securityManager.validateIP(remoteIP); if (!ipValidation.allowed) { logger.log('warn', `Connection rejected`, { remoteIP, reason: ipValidation.reason, component: 'route-handler' }); - socket.end(); - socket.destroy(); + cleanupSocket(socket, `rejected-${ipValidation.reason}`); return; } @@ -825,16 +824,16 @@ export class RouteConnectionHandler { // Track event listeners added by the handler so we can clean them up const originalOn = socket.on.bind(socket); const originalOnce = socket.once.bind(socket); - const trackedListeners: Array<{event: string; listener: Function}> = []; + const trackedListeners: Array<{event: string; listener: (...args: any[]) => void}> = []; // Override socket.on to track listeners - socket.on = function(event: string, listener: Function) { + socket.on = function(event: string, listener: (...args: any[]) => void) { trackedListeners.push({event, listener}); return originalOn(event, listener); } as any; // Override socket.once to track listeners - socket.once = function(event: string, listener: Function) { + socket.once = function(event: string, listener: (...args: any[]) => void) { trackedListeners.push({event, listener}); return originalOnce(event, listener); } as any; @@ -1265,7 +1264,7 @@ export class RouteConnectionHandler { connectionId, serverName, connInfo, - (connectionId, reason) => this.connectionManager.initiateCleanupOnce(record, reason) + (_connectionId, reason) => this.connectionManager.initiateCleanupOnce(record, reason) ); // Store the handler in the connection record so we can remove it during cleanup