Compare commits
5 Commits
Author | SHA1 | Date | |
---|---|---|---|
3ac3345be8 | |||
5b40e82c41 | |||
2a75a86d73 | |||
250eafd36c | |||
facb68a9d0 |
@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@push.rocks/smartproxy",
|
||||
"version": "19.5.14",
|
||||
"version": "19.5.17",
|
||||
"private": false,
|
||||
"description": "A powerful proxy package with unified route-based configuration for high traffic management. Features include SSL/TLS support, flexible routing patterns, WebSocket handling, advanced security options, and automatic ACME certificate management.",
|
||||
"main": "dist_ts/index.js",
|
||||
|
120
readme.hints.md
120
readme.hints.md
@ -464,4 +464,122 @@ The fix was applied in two places:
|
||||
1. **ForwardingHandler classes** (`https-passthrough-handler.ts`, etc.) - These are standalone forwarding utilities
|
||||
2. **SmartProxy route-connection-handler** (`route-connection-handler.ts`) - This is where the actual SmartProxy connection handling happens
|
||||
|
||||
The critical fix for SmartProxy was in `setupDirectConnection()` method in route-connection-handler.ts, which now uses `createSocketWithErrorHandler()` to properly handle connection failures and clean up connection records.
|
||||
The critical fix for SmartProxy was in `setupDirectConnection()` method in route-connection-handler.ts, which now uses `createSocketWithErrorHandler()` to properly handle connection failures and clean up connection records.
|
||||
|
||||
## Connection Cleanup Improvements (v19.5.12+)
|
||||
|
||||
### Issue
|
||||
Connections were still counting up during rapid retry scenarios, especially when routing failed or backend connections were refused. This was due to:
|
||||
1. **Delayed Cleanup**: Using `initiateCleanupOnce` queued cleanup operations (batch of 100 every 100ms) instead of immediate cleanup
|
||||
2. **NFTables Memory Leak**: NFTables connections were never cleaned up, staying in memory forever
|
||||
3. **Connection Limit Bypass**: When max connections reached, connection record check happened after creation
|
||||
|
||||
### Root Cause Analysis
|
||||
1. **Queued vs Immediate Cleanup**:
|
||||
- `initiateCleanupOnce()`: Adds to cleanup queue, processes up to 100 connections every 100ms
|
||||
- `cleanupConnection()`: Immediate synchronous cleanup
|
||||
- Under rapid retries, connections were created faster than the queue could process them
|
||||
|
||||
2. **NFTables Connections**:
|
||||
- Marked with `usingNetworkProxy = true` but never cleaned up
|
||||
- Connection records stayed in memory indefinitely
|
||||
|
||||
3. **Error Path Cleanup**:
|
||||
- Many error paths used `socket.end()` (async) followed by cleanup
|
||||
- Created timing windows where connections weren't fully cleaned
|
||||
|
||||
### Solution
|
||||
1. **Immediate Cleanup**: Changed all error paths from `initiateCleanupOnce()` to `cleanupConnection()` for immediate cleanup
|
||||
2. **NFTables Cleanup**: Added socket close listener to clean up connection records when NFTables connections close
|
||||
3. **Connection Limit Fix**: Added null check after `createConnection()` to handle rejection properly
|
||||
|
||||
### Changes Made in route-connection-handler.ts
|
||||
```typescript
|
||||
// 1. NFTables cleanup (line 551-553)
|
||||
socket.once('close', () => {
|
||||
this.connectionManager.cleanupConnection(record, 'nftables_closed');
|
||||
});
|
||||
|
||||
// 2. Connection limit check (line 93-96)
|
||||
const record = this.connectionManager.createConnection(socket);
|
||||
if (!record) {
|
||||
// Connection was rejected due to limit - socket already destroyed
|
||||
return;
|
||||
}
|
||||
|
||||
// 3. Changed all error paths to use immediate cleanup
|
||||
// Before: this.connectionManager.initiateCleanupOnce(record, reason)
|
||||
// After: this.connectionManager.cleanupConnection(record, reason)
|
||||
```
|
||||
|
||||
### Test Coverage
|
||||
- `test/test.rapid-retry-cleanup.node.ts` - Verifies connection cleanup under rapid retry scenarios
|
||||
- Test shows connection count stays at 0 even with 20 rapid retries with 50ms intervals
|
||||
- Confirms both ECONNREFUSED and routing failure scenarios are handled correctly
|
||||
|
||||
### Performance Impact
|
||||
- **Positive**: No more connection accumulation under load
|
||||
- **Positive**: Immediate cleanup reduces memory usage
|
||||
- **Consideration**: More frequent cleanup operations, but prevents queue backlog
|
||||
|
||||
### Migration Notes
|
||||
No configuration changes needed. The improvements are automatic and backward compatible.
|
||||
|
||||
## Early Client Disconnect Handling (v19.5.13+)
|
||||
|
||||
### Issue
|
||||
Connections were accumulating when clients connected but disconnected before sending data or during routing. This occurred in two scenarios:
|
||||
1. **TLS Path**: Clients connecting and disconnecting before sending initial TLS handshake data
|
||||
2. **Non-TLS Immediate Routing**: Clients disconnecting while backend connection was being established
|
||||
|
||||
### Root Cause
|
||||
1. **Missing Cleanup Handlers**: During initial data wait and immediate routing, no close/end handlers were attached to catch early disconnections
|
||||
2. **Race Condition**: Backend connection attempts continued even after client disconnected, causing unhandled errors
|
||||
3. **Timing Window**: Between accepting connection and establishing full bidirectional flow, disconnections weren't properly handled
|
||||
|
||||
### Solution
|
||||
1. **TLS Path Fix**: Added close/end handlers during initial data wait (lines 224-253 in route-connection-handler.ts)
|
||||
2. **Immediate Routing Fix**: Used `setupSocketHandlers` for proper handler attachment (lines 180-205)
|
||||
3. **Backend Error Handling**: Check if connection already closed before handling backend errors (line 1144)
|
||||
|
||||
### Changes Made
|
||||
```typescript
|
||||
// 1. TLS path - handle disconnect before initial data
|
||||
socket.once('close', () => {
|
||||
if (!initialDataReceived) {
|
||||
this.connectionManager.cleanupConnection(record, 'closed_before_data');
|
||||
}
|
||||
});
|
||||
|
||||
// 2. Immediate routing path - proper handler setup
|
||||
setupSocketHandlers(socket, (reason) => {
|
||||
if (!record.outgoing || record.outgoing.readyState !== 'open') {
|
||||
if (record.outgoing && !record.outgoing.destroyed) {
|
||||
record.outgoing.destroy(); // Abort pending backend connection
|
||||
}
|
||||
this.connectionManager.cleanupConnection(record, reason);
|
||||
}
|
||||
}, undefined, 'immediate-route-client');
|
||||
|
||||
// 3. Backend connection error handling
|
||||
onError: (error) => {
|
||||
if (record.connectionClosed) {
|
||||
logger.log('debug', 'Backend connection failed but client already disconnected');
|
||||
return; // Client already gone, nothing to clean up
|
||||
}
|
||||
// ... normal error handling
|
||||
}
|
||||
```
|
||||
|
||||
### Test Coverage
|
||||
- `test/test.connect-disconnect-cleanup.node.ts` - Comprehensive test for early disconnect scenarios
|
||||
- Tests verify connection count stays at 0 even with rapid connect/disconnect patterns
|
||||
- Covers immediate disconnect, delayed disconnect, and mixed patterns
|
||||
|
||||
### Performance Impact
|
||||
- **Positive**: No more connection accumulation from early disconnects
|
||||
- **Positive**: Immediate cleanup reduces memory usage
|
||||
- **Positive**: Prevents resource exhaustion from rapid reconnection attempts
|
||||
|
||||
### Migration Notes
|
||||
No configuration changes needed. The fix is automatic and backward compatible.
|
242
test/test.connect-disconnect-cleanup.node.ts
Normal file
242
test/test.connect-disconnect-cleanup.node.ts
Normal file
@ -0,0 +1,242 @@
|
||||
import { tap, expect } from '@git.zone/tstest/tapbundle';
|
||||
import * as net from 'net';
|
||||
import * as plugins from '../ts/plugins.js';
|
||||
|
||||
// Import SmartProxy and configurations
|
||||
import { SmartProxy } from '../ts/index.js';
|
||||
|
||||
tap.test('should handle clients that connect and immediately disconnect without sending data', async () => {
|
||||
console.log('\n=== Testing Connect-Disconnect Cleanup ===');
|
||||
|
||||
// Create a SmartProxy instance
|
||||
const proxy = new SmartProxy({
|
||||
ports: [8560],
|
||||
enableDetailedLogging: false,
|
||||
initialDataTimeout: 5000, // 5 second timeout for initial data
|
||||
routes: [{
|
||||
name: 'test-route',
|
||||
match: { ports: 8560 },
|
||||
action: {
|
||||
type: 'forward',
|
||||
target: {
|
||||
host: 'localhost',
|
||||
port: 9999 // Non-existent port
|
||||
}
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
// Start the proxy
|
||||
await proxy.start();
|
||||
console.log('✓ Proxy started on port 8560');
|
||||
|
||||
// Helper to get active connection count
|
||||
const getActiveConnections = () => {
|
||||
const connectionManager = (proxy as any).connectionManager;
|
||||
return connectionManager ? connectionManager.getConnectionCount() : 0;
|
||||
};
|
||||
|
||||
const initialCount = getActiveConnections();
|
||||
console.log(`Initial connection count: ${initialCount}`);
|
||||
|
||||
// Test 1: Connect and immediately disconnect without sending data
|
||||
console.log('\n--- Test 1: Immediate disconnect ---');
|
||||
const connectionCounts: number[] = [];
|
||||
|
||||
for (let i = 0; i < 10; i++) {
|
||||
const client = new net.Socket();
|
||||
|
||||
// Connect and immediately destroy
|
||||
client.connect(8560, 'localhost', () => {
|
||||
// Connected - immediately destroy without sending data
|
||||
client.destroy();
|
||||
});
|
||||
|
||||
// Wait a tiny bit
|
||||
await new Promise(resolve => setTimeout(resolve, 10));
|
||||
|
||||
const count = getActiveConnections();
|
||||
connectionCounts.push(count);
|
||||
if ((i + 1) % 5 === 0) {
|
||||
console.log(`After ${i + 1} connect/disconnect cycles: ${count} active connections`);
|
||||
}
|
||||
}
|
||||
|
||||
// Wait a bit for cleanup
|
||||
await new Promise(resolve => setTimeout(resolve, 500));
|
||||
|
||||
const afterImmediateDisconnect = getActiveConnections();
|
||||
console.log(`After immediate disconnect test: ${afterImmediateDisconnect} active connections`);
|
||||
|
||||
// Test 2: Connect, wait a bit, then disconnect without sending data
|
||||
console.log('\n--- Test 2: Delayed disconnect ---');
|
||||
|
||||
for (let i = 0; i < 5; i++) {
|
||||
const client = new net.Socket();
|
||||
|
||||
client.on('error', () => {
|
||||
// Ignore errors
|
||||
});
|
||||
|
||||
client.connect(8560, 'localhost', () => {
|
||||
// Wait 100ms then disconnect without sending data
|
||||
setTimeout(() => {
|
||||
if (!client.destroyed) {
|
||||
client.destroy();
|
||||
}
|
||||
}, 100);
|
||||
});
|
||||
}
|
||||
|
||||
// Check count immediately
|
||||
const duringDelayed = getActiveConnections();
|
||||
console.log(`During delayed disconnect test: ${duringDelayed} active connections`);
|
||||
|
||||
// Wait for cleanup
|
||||
await new Promise(resolve => setTimeout(resolve, 1000));
|
||||
|
||||
const afterDelayedDisconnect = getActiveConnections();
|
||||
console.log(`After delayed disconnect test: ${afterDelayedDisconnect} active connections`);
|
||||
|
||||
// Test 3: Mix of immediate and delayed disconnects
|
||||
console.log('\n--- Test 3: Mixed disconnect patterns ---');
|
||||
|
||||
const promises = [];
|
||||
for (let i = 0; i < 20; i++) {
|
||||
promises.push(new Promise<void>((resolve) => {
|
||||
const client = new net.Socket();
|
||||
|
||||
client.on('error', () => {
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.on('close', () => {
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.connect(8560, 'localhost', () => {
|
||||
if (i % 2 === 0) {
|
||||
// Half disconnect immediately
|
||||
client.destroy();
|
||||
} else {
|
||||
// Half wait 50ms
|
||||
setTimeout(() => {
|
||||
if (!client.destroyed) {
|
||||
client.destroy();
|
||||
}
|
||||
}, 50);
|
||||
}
|
||||
});
|
||||
|
||||
// Failsafe timeout
|
||||
setTimeout(() => resolve(), 200);
|
||||
}));
|
||||
}
|
||||
|
||||
// Wait for all to complete
|
||||
await Promise.all(promises);
|
||||
|
||||
const duringMixed = getActiveConnections();
|
||||
console.log(`During mixed test: ${duringMixed} active connections`);
|
||||
|
||||
// Final cleanup wait
|
||||
await new Promise(resolve => setTimeout(resolve, 1000));
|
||||
|
||||
const finalCount = getActiveConnections();
|
||||
console.log(`\nFinal connection count: ${finalCount}`);
|
||||
|
||||
// Stop the proxy
|
||||
await proxy.stop();
|
||||
console.log('✓ Proxy stopped');
|
||||
|
||||
// Verify all connections were cleaned up
|
||||
expect(finalCount).toEqual(initialCount);
|
||||
expect(afterImmediateDisconnect).toEqual(initialCount);
|
||||
expect(afterDelayedDisconnect).toEqual(initialCount);
|
||||
|
||||
// Check that connections didn't accumulate during the test
|
||||
const maxCount = Math.max(...connectionCounts);
|
||||
console.log(`\nMax connection count during immediate disconnect test: ${maxCount}`);
|
||||
expect(maxCount).toBeLessThan(3); // Should stay very low
|
||||
|
||||
console.log('\n✅ PASS: Connect-disconnect cleanup working correctly!');
|
||||
});
|
||||
|
||||
tap.test('should handle clients that error during connection', async () => {
|
||||
console.log('\n=== Testing Connection Error Cleanup ===');
|
||||
|
||||
const proxy = new SmartProxy({
|
||||
ports: [8561],
|
||||
enableDetailedLogging: false,
|
||||
routes: [{
|
||||
name: 'test-route',
|
||||
match: { ports: 8561 },
|
||||
action: {
|
||||
type: 'forward',
|
||||
target: {
|
||||
host: 'localhost',
|
||||
port: 9999
|
||||
}
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
await proxy.start();
|
||||
console.log('✓ Proxy started on port 8561');
|
||||
|
||||
const getActiveConnections = () => {
|
||||
const connectionManager = (proxy as any).connectionManager;
|
||||
return connectionManager ? connectionManager.getConnectionCount() : 0;
|
||||
};
|
||||
|
||||
const initialCount = getActiveConnections();
|
||||
console.log(`Initial connection count: ${initialCount}`);
|
||||
|
||||
// Create connections that will error
|
||||
const promises = [];
|
||||
for (let i = 0; i < 10; i++) {
|
||||
promises.push(new Promise<void>((resolve) => {
|
||||
const client = new net.Socket();
|
||||
|
||||
client.on('error', () => {
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.on('close', () => {
|
||||
resolve();
|
||||
});
|
||||
|
||||
// Connect to proxy
|
||||
client.connect(8561, 'localhost', () => {
|
||||
// Force an error by writing invalid data then destroying
|
||||
try {
|
||||
client.write(Buffer.alloc(1024 * 1024)); // Large write
|
||||
client.destroy();
|
||||
} catch (e) {
|
||||
// Ignore
|
||||
}
|
||||
});
|
||||
|
||||
// Timeout
|
||||
setTimeout(() => resolve(), 500);
|
||||
}));
|
||||
}
|
||||
|
||||
await Promise.all(promises);
|
||||
console.log('✓ All error connections completed');
|
||||
|
||||
// Wait for cleanup
|
||||
await new Promise(resolve => setTimeout(resolve, 500));
|
||||
|
||||
const finalCount = getActiveConnections();
|
||||
console.log(`Final connection count: ${finalCount}`);
|
||||
|
||||
await proxy.stop();
|
||||
console.log('✓ Proxy stopped');
|
||||
|
||||
expect(finalCount).toEqual(initialCount);
|
||||
|
||||
console.log('\n✅ PASS: Connection error cleanup working correctly!');
|
||||
});
|
||||
|
||||
tap.start();
|
279
test/test.connection-cleanup-comprehensive.node.ts
Normal file
279
test/test.connection-cleanup-comprehensive.node.ts
Normal file
@ -0,0 +1,279 @@
|
||||
import { tap, expect } from '@git.zone/tstest/tapbundle';
|
||||
import * as net from 'net';
|
||||
import * as plugins from '../ts/plugins.js';
|
||||
|
||||
// Import SmartProxy and configurations
|
||||
import { SmartProxy } from '../ts/index.js';
|
||||
|
||||
tap.test('comprehensive connection cleanup test - all scenarios', async () => {
|
||||
console.log('\n=== Comprehensive Connection Cleanup Test ===');
|
||||
|
||||
// Create a SmartProxy instance
|
||||
const proxy = new SmartProxy({
|
||||
ports: [8570, 8571], // One for immediate routing, one for TLS
|
||||
enableDetailedLogging: false,
|
||||
initialDataTimeout: 2000,
|
||||
socketTimeout: 5000,
|
||||
routes: [
|
||||
{
|
||||
name: 'non-tls-route',
|
||||
match: { ports: 8570 },
|
||||
action: {
|
||||
type: 'forward',
|
||||
target: {
|
||||
host: 'localhost',
|
||||
port: 9999 // Non-existent port
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
name: 'tls-route',
|
||||
match: { ports: 8571 },
|
||||
action: {
|
||||
type: 'forward',
|
||||
target: {
|
||||
host: 'localhost',
|
||||
port: 9999 // Non-existent port
|
||||
},
|
||||
tls: {
|
||||
mode: 'passthrough'
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
});
|
||||
|
||||
// Start the proxy
|
||||
await proxy.start();
|
||||
console.log('✓ Proxy started on ports 8570 (non-TLS) and 8571 (TLS)');
|
||||
|
||||
// Helper to get active connection count
|
||||
const getActiveConnections = () => {
|
||||
const connectionManager = (proxy as any).connectionManager;
|
||||
return connectionManager ? connectionManager.getConnectionCount() : 0;
|
||||
};
|
||||
|
||||
const initialCount = getActiveConnections();
|
||||
console.log(`Initial connection count: ${initialCount}`);
|
||||
|
||||
// Test 1: Rapid ECONNREFUSED retries (from original issue)
|
||||
console.log('\n--- Test 1: Rapid ECONNREFUSED retries ---');
|
||||
for (let i = 0; i < 10; i++) {
|
||||
await new Promise<void>((resolve) => {
|
||||
const client = new net.Socket();
|
||||
|
||||
client.on('error', () => {
|
||||
client.destroy();
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.on('close', () => {
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.connect(8570, 'localhost', () => {
|
||||
// Send data to trigger routing
|
||||
client.write('GET / HTTP/1.1\r\nHost: test.com\r\n\r\n');
|
||||
});
|
||||
|
||||
setTimeout(() => {
|
||||
if (!client.destroyed) {
|
||||
client.destroy();
|
||||
}
|
||||
resolve();
|
||||
}, 100);
|
||||
});
|
||||
|
||||
if ((i + 1) % 5 === 0) {
|
||||
const count = getActiveConnections();
|
||||
console.log(`After ${i + 1} ECONNREFUSED retries: ${count} active connections`);
|
||||
}
|
||||
}
|
||||
|
||||
// Test 2: Connect without sending data (immediate disconnect)
|
||||
console.log('\n--- Test 2: Connect without sending data ---');
|
||||
for (let i = 0; i < 10; i++) {
|
||||
const client = new net.Socket();
|
||||
|
||||
client.on('error', () => {
|
||||
// Ignore
|
||||
});
|
||||
|
||||
// Connect to non-TLS port and immediately disconnect
|
||||
client.connect(8570, 'localhost', () => {
|
||||
client.destroy();
|
||||
});
|
||||
|
||||
await new Promise(resolve => setTimeout(resolve, 10));
|
||||
}
|
||||
|
||||
const afterNoData = getActiveConnections();
|
||||
console.log(`After connect-without-data test: ${afterNoData} active connections`);
|
||||
|
||||
// Test 3: TLS connections that disconnect before handshake
|
||||
console.log('\n--- Test 3: TLS early disconnect ---');
|
||||
for (let i = 0; i < 10; i++) {
|
||||
const client = new net.Socket();
|
||||
|
||||
client.on('error', () => {
|
||||
// Ignore
|
||||
});
|
||||
|
||||
// Connect to TLS port but disconnect before sending handshake
|
||||
client.connect(8571, 'localhost', () => {
|
||||
// Wait 50ms then disconnect (before initial data timeout)
|
||||
setTimeout(() => {
|
||||
client.destroy();
|
||||
}, 50);
|
||||
});
|
||||
|
||||
await new Promise(resolve => setTimeout(resolve, 100));
|
||||
}
|
||||
|
||||
const afterTlsEarly = getActiveConnections();
|
||||
console.log(`After TLS early disconnect test: ${afterTlsEarly} active connections`);
|
||||
|
||||
// Test 4: Mixed pattern - simulating real-world chaos
|
||||
console.log('\n--- Test 4: Mixed chaos pattern ---');
|
||||
const promises = [];
|
||||
|
||||
for (let i = 0; i < 30; i++) {
|
||||
promises.push(new Promise<void>((resolve) => {
|
||||
const client = new net.Socket();
|
||||
const port = i % 2 === 0 ? 8570 : 8571;
|
||||
|
||||
client.on('error', () => {
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.on('close', () => {
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.connect(port, 'localhost', () => {
|
||||
const scenario = i % 5;
|
||||
|
||||
switch (scenario) {
|
||||
case 0:
|
||||
// Immediate disconnect
|
||||
client.destroy();
|
||||
break;
|
||||
case 1:
|
||||
// Send data then disconnect
|
||||
client.write('GET / HTTP/1.1\r\nHost: test.com\r\n\r\n');
|
||||
setTimeout(() => client.destroy(), 20);
|
||||
break;
|
||||
case 2:
|
||||
// Disconnect after delay
|
||||
setTimeout(() => client.destroy(), 100);
|
||||
break;
|
||||
case 3:
|
||||
// Send partial TLS handshake
|
||||
if (port === 8571) {
|
||||
client.write(Buffer.from([0x16, 0x03, 0x01])); // Partial TLS
|
||||
}
|
||||
setTimeout(() => client.destroy(), 50);
|
||||
break;
|
||||
case 4:
|
||||
// Just let it timeout
|
||||
break;
|
||||
}
|
||||
});
|
||||
|
||||
// Failsafe
|
||||
setTimeout(() => {
|
||||
if (!client.destroyed) {
|
||||
client.destroy();
|
||||
}
|
||||
resolve();
|
||||
}, 500);
|
||||
}));
|
||||
|
||||
// Small delay between connections
|
||||
if (i % 5 === 0) {
|
||||
await new Promise(resolve => setTimeout(resolve, 10));
|
||||
}
|
||||
}
|
||||
|
||||
await Promise.all(promises);
|
||||
console.log('✓ Chaos test completed');
|
||||
|
||||
// Wait for any cleanup
|
||||
await new Promise(resolve => setTimeout(resolve, 1000));
|
||||
|
||||
const afterChaos = getActiveConnections();
|
||||
console.log(`After chaos test: ${afterChaos} active connections`);
|
||||
|
||||
// Test 5: NFTables route (should cleanup properly)
|
||||
console.log('\n--- Test 5: NFTables route cleanup ---');
|
||||
const nftProxy = new SmartProxy({
|
||||
ports: [8572],
|
||||
enableDetailedLogging: false,
|
||||
routes: [{
|
||||
name: 'nftables-route',
|
||||
match: { ports: 8572 },
|
||||
action: {
|
||||
type: 'forward',
|
||||
forwardingEngine: 'nftables',
|
||||
target: {
|
||||
host: 'localhost',
|
||||
port: 9999
|
||||
}
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
await nftProxy.start();
|
||||
|
||||
const getNftConnections = () => {
|
||||
const connectionManager = (nftProxy as any).connectionManager;
|
||||
return connectionManager ? connectionManager.getConnectionCount() : 0;
|
||||
};
|
||||
|
||||
// Create NFTables connections
|
||||
for (let i = 0; i < 5; i++) {
|
||||
const client = new net.Socket();
|
||||
|
||||
client.on('error', () => {
|
||||
// Ignore
|
||||
});
|
||||
|
||||
client.connect(8572, 'localhost', () => {
|
||||
setTimeout(() => client.destroy(), 50);
|
||||
});
|
||||
|
||||
await new Promise(resolve => setTimeout(resolve, 100));
|
||||
}
|
||||
|
||||
await new Promise(resolve => setTimeout(resolve, 500));
|
||||
|
||||
const nftFinal = getNftConnections();
|
||||
console.log(`NFTables connections after test: ${nftFinal}`);
|
||||
|
||||
await nftProxy.stop();
|
||||
|
||||
// Final check on main proxy
|
||||
const finalCount = getActiveConnections();
|
||||
console.log(`\nFinal connection count: ${finalCount}`);
|
||||
|
||||
// Stop the proxy
|
||||
await proxy.stop();
|
||||
console.log('✓ Proxy stopped');
|
||||
|
||||
// Verify all connections were cleaned up
|
||||
expect(finalCount).toEqual(initialCount);
|
||||
expect(afterNoData).toEqual(initialCount);
|
||||
expect(afterTlsEarly).toEqual(initialCount);
|
||||
expect(afterChaos).toEqual(initialCount);
|
||||
expect(nftFinal).toEqual(0);
|
||||
|
||||
console.log('\n✅ PASS: Comprehensive connection cleanup test passed!');
|
||||
console.log('All connection scenarios properly cleaned up:');
|
||||
console.log('- ECONNREFUSED rapid retries');
|
||||
console.log('- Connect without sending data');
|
||||
console.log('- TLS early disconnect');
|
||||
console.log('- Mixed chaos patterns');
|
||||
console.log('- NFTables connections');
|
||||
});
|
||||
|
||||
tap.start();
|
201
test/test.rapid-retry-cleanup.node.ts
Normal file
201
test/test.rapid-retry-cleanup.node.ts
Normal file
@ -0,0 +1,201 @@
|
||||
import { tap, expect } from '@git.zone/tstest/tapbundle';
|
||||
import * as net from 'net';
|
||||
import * as plugins from '../ts/plugins.js';
|
||||
|
||||
// Import SmartProxy and configurations
|
||||
import { SmartProxy } from '../ts/index.js';
|
||||
|
||||
tap.test('should handle rapid connection retries without leaking connections', async () => {
|
||||
console.log('\n=== Testing Rapid Connection Retry Cleanup ===');
|
||||
|
||||
// Create a SmartProxy instance
|
||||
const proxy = new SmartProxy({
|
||||
ports: [8550],
|
||||
enableDetailedLogging: false,
|
||||
maxConnectionLifetime: 10000,
|
||||
socketTimeout: 5000,
|
||||
routes: [{
|
||||
name: 'test-route',
|
||||
match: { ports: 8550 },
|
||||
action: {
|
||||
type: 'forward',
|
||||
target: {
|
||||
host: 'localhost',
|
||||
port: 9999 // Non-existent port to force connection failures
|
||||
}
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
// Start the proxy
|
||||
await proxy.start();
|
||||
console.log('✓ Proxy started on port 8550');
|
||||
|
||||
// Helper to get active connection count
|
||||
const getActiveConnections = () => {
|
||||
const connectionManager = (proxy as any).connectionManager;
|
||||
return connectionManager ? connectionManager.getConnectionCount() : 0;
|
||||
};
|
||||
|
||||
// Track connection counts
|
||||
const connectionCounts: number[] = [];
|
||||
const initialCount = getActiveConnections();
|
||||
console.log(`Initial connection count: ${initialCount}`);
|
||||
|
||||
// Simulate rapid retries
|
||||
const retryCount = 20;
|
||||
const retryDelay = 50; // 50ms between retries
|
||||
let successfulConnections = 0;
|
||||
let failedConnections = 0;
|
||||
|
||||
console.log(`\nSimulating ${retryCount} rapid connection attempts...`);
|
||||
|
||||
for (let i = 0; i < retryCount; i++) {
|
||||
await new Promise<void>((resolve) => {
|
||||
const client = new net.Socket();
|
||||
|
||||
client.on('error', () => {
|
||||
failedConnections++;
|
||||
client.destroy();
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.on('close', () => {
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.connect(8550, 'localhost', () => {
|
||||
// Send some data to trigger routing
|
||||
client.write('GET / HTTP/1.1\r\nHost: test.com\r\n\r\n');
|
||||
successfulConnections++;
|
||||
});
|
||||
|
||||
// Force close after a short time
|
||||
setTimeout(() => {
|
||||
if (!client.destroyed) {
|
||||
client.destroy();
|
||||
}
|
||||
}, 100);
|
||||
});
|
||||
|
||||
// Small delay between retries
|
||||
await new Promise(resolve => setTimeout(resolve, retryDelay));
|
||||
|
||||
// Check connection count after each attempt
|
||||
const currentCount = getActiveConnections();
|
||||
connectionCounts.push(currentCount);
|
||||
|
||||
if ((i + 1) % 5 === 0) {
|
||||
console.log(`After ${i + 1} attempts: ${currentCount} active connections`);
|
||||
}
|
||||
}
|
||||
|
||||
console.log(`\nConnection attempts complete:`);
|
||||
console.log(`- Successful: ${successfulConnections}`);
|
||||
console.log(`- Failed: ${failedConnections}`);
|
||||
|
||||
// Wait a bit for any pending cleanups
|
||||
console.log('\nWaiting for cleanup...');
|
||||
await new Promise(resolve => setTimeout(resolve, 1000));
|
||||
|
||||
// Check final connection count
|
||||
const finalCount = getActiveConnections();
|
||||
console.log(`\nFinal connection count: ${finalCount}`);
|
||||
|
||||
// Analyze connection count trend
|
||||
const maxCount = Math.max(...connectionCounts);
|
||||
const avgCount = connectionCounts.reduce((a, b) => a + b, 0) / connectionCounts.length;
|
||||
|
||||
console.log(`\nConnection count statistics:`);
|
||||
console.log(`- Maximum: ${maxCount}`);
|
||||
console.log(`- Average: ${avgCount.toFixed(2)}`);
|
||||
console.log(`- Initial: ${initialCount}`);
|
||||
console.log(`- Final: ${finalCount}`);
|
||||
|
||||
// Stop the proxy
|
||||
await proxy.stop();
|
||||
console.log('\n✓ Proxy stopped');
|
||||
|
||||
// Verify results
|
||||
expect(finalCount).toEqual(initialCount);
|
||||
expect(maxCount).toBeLessThan(10); // Should not accumulate many connections
|
||||
|
||||
console.log('\n✅ PASS: Connection cleanup working correctly under rapid retries!');
|
||||
});
|
||||
|
||||
tap.test('should handle routing failures without leaking connections', async () => {
|
||||
console.log('\n=== Testing Routing Failure Cleanup ===');
|
||||
|
||||
// Create a SmartProxy instance with no routes
|
||||
const proxy = new SmartProxy({
|
||||
ports: [8551],
|
||||
enableDetailedLogging: false,
|
||||
maxConnectionLifetime: 10000,
|
||||
socketTimeout: 5000,
|
||||
routes: [] // No routes - all connections will fail routing
|
||||
});
|
||||
|
||||
// Start the proxy
|
||||
await proxy.start();
|
||||
console.log('✓ Proxy started on port 8551 with no routes');
|
||||
|
||||
// Helper to get active connection count
|
||||
const getActiveConnections = () => {
|
||||
const connectionManager = (proxy as any).connectionManager;
|
||||
return connectionManager ? connectionManager.getConnectionCount() : 0;
|
||||
};
|
||||
|
||||
const initialCount = getActiveConnections();
|
||||
console.log(`Initial connection count: ${initialCount}`);
|
||||
|
||||
// Create multiple connections that will fail routing
|
||||
const connectionPromises = [];
|
||||
for (let i = 0; i < 10; i++) {
|
||||
connectionPromises.push(new Promise<void>((resolve) => {
|
||||
const client = new net.Socket();
|
||||
|
||||
client.on('error', () => {
|
||||
client.destroy();
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.on('close', () => {
|
||||
resolve();
|
||||
});
|
||||
|
||||
client.connect(8551, 'localhost', () => {
|
||||
// Send data to trigger routing (which will fail)
|
||||
client.write('GET / HTTP/1.1\r\nHost: test.com\r\n\r\n');
|
||||
});
|
||||
|
||||
// Force close after a short time
|
||||
setTimeout(() => {
|
||||
if (!client.destroyed) {
|
||||
client.destroy();
|
||||
}
|
||||
resolve();
|
||||
}, 500);
|
||||
}));
|
||||
}
|
||||
|
||||
// Wait for all connections to complete
|
||||
await Promise.all(connectionPromises);
|
||||
console.log('✓ All connection attempts completed');
|
||||
|
||||
// Wait for cleanup
|
||||
await new Promise(resolve => setTimeout(resolve, 500));
|
||||
|
||||
const finalCount = getActiveConnections();
|
||||
console.log(`Final connection count: ${finalCount}`);
|
||||
|
||||
// Stop the proxy
|
||||
await proxy.stop();
|
||||
console.log('✓ Proxy stopped');
|
||||
|
||||
// Verify no connections leaked
|
||||
expect(finalCount).toEqual(initialCount);
|
||||
|
||||
console.log('\n✅ PASS: Routing failures cleaned up correctly!');
|
||||
});
|
||||
|
||||
tap.start();
|
@ -90,6 +90,10 @@ export class RouteConnectionHandler {
|
||||
|
||||
// Create a new connection record
|
||||
const record = this.connectionManager.createConnection(socket);
|
||||
if (!record) {
|
||||
// Connection was rejected due to limit - socket already destroyed by connection manager
|
||||
return;
|
||||
}
|
||||
const connectionId = record.id;
|
||||
|
||||
// Apply socket optimizations
|
||||
@ -172,8 +176,33 @@ export class RouteConnectionHandler {
|
||||
|
||||
// If no routes require TLS handling and it's not port 443, route immediately
|
||||
if (!needsTlsHandling && localPort !== 443) {
|
||||
// Set up error handler
|
||||
socket.on('error', this.connectionManager.handleError('incoming', record));
|
||||
// Set up proper socket handlers for immediate routing
|
||||
setupSocketHandlers(
|
||||
socket,
|
||||
(reason) => {
|
||||
// Only cleanup if connection hasn't been fully established
|
||||
// Check if outgoing connection exists and is connected
|
||||
if (!record.outgoing || record.outgoing.readyState !== 'open') {
|
||||
logger.log('debug', `Connection ${connectionId} closed during immediate routing: ${reason}`, {
|
||||
connectionId,
|
||||
remoteIP: record.remoteIP,
|
||||
reason,
|
||||
hasOutgoing: !!record.outgoing,
|
||||
outgoingState: record.outgoing?.readyState,
|
||||
component: 'route-handler'
|
||||
});
|
||||
|
||||
// If there's a pending outgoing connection, destroy it
|
||||
if (record.outgoing && !record.outgoing.destroyed) {
|
||||
record.outgoing.destroy();
|
||||
}
|
||||
|
||||
this.connectionManager.cleanupConnection(record, reason);
|
||||
}
|
||||
},
|
||||
undefined, // Use default timeout handler
|
||||
'immediate-route-client'
|
||||
);
|
||||
|
||||
// Route immediately for non-TLS connections
|
||||
this.routeConnection(socket, record, '', undefined);
|
||||
@ -217,6 +246,37 @@ export class RouteConnectionHandler {
|
||||
// Set up error handler
|
||||
socket.on('error', this.connectionManager.handleError('incoming', record));
|
||||
|
||||
// Add close/end handlers to catch immediate disconnections
|
||||
socket.once('close', () => {
|
||||
if (!initialDataReceived) {
|
||||
logger.log('warn', `Connection ${connectionId} closed before sending initial data`, {
|
||||
connectionId,
|
||||
remoteIP: record.remoteIP,
|
||||
component: 'route-handler'
|
||||
});
|
||||
if (initialTimeout) {
|
||||
clearTimeout(initialTimeout);
|
||||
initialTimeout = null;
|
||||
}
|
||||
this.connectionManager.cleanupConnection(record, 'closed_before_data');
|
||||
}
|
||||
});
|
||||
|
||||
socket.once('end', () => {
|
||||
if (!initialDataReceived) {
|
||||
logger.log('debug', `Connection ${connectionId} ended before sending initial data`, {
|
||||
connectionId,
|
||||
remoteIP: record.remoteIP,
|
||||
component: 'route-handler'
|
||||
});
|
||||
if (initialTimeout) {
|
||||
clearTimeout(initialTimeout);
|
||||
initialTimeout = null;
|
||||
}
|
||||
// Don't cleanup on 'end' - wait for 'close'
|
||||
}
|
||||
});
|
||||
|
||||
// First data handler to capture initial TLS handshake
|
||||
socket.once('data', (chunk: Buffer) => {
|
||||
// Clear the initial timeout since we've received data
|
||||
@ -546,6 +606,12 @@ export class RouteConnectionHandler {
|
||||
|
||||
// We don't close the socket - just let it remain open
|
||||
// The kernel-level NFTables rules will handle the actual forwarding
|
||||
|
||||
// Set up cleanup when the socket eventually closes
|
||||
socket.once('close', () => {
|
||||
this.connectionManager.cleanupConnection(record, 'nftables_closed');
|
||||
});
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
@ -687,7 +753,7 @@ export class RouteConnectionHandler {
|
||||
record,
|
||||
initialChunk,
|
||||
this.settings.httpProxyPort || 8443,
|
||||
(reason) => this.connectionManager.initiateCleanupOnce(record, reason)
|
||||
(reason) => this.connectionManager.cleanupConnection(record, reason)
|
||||
);
|
||||
return;
|
||||
}
|
||||
@ -742,7 +808,7 @@ export class RouteConnectionHandler {
|
||||
record,
|
||||
initialChunk,
|
||||
this.settings.httpProxyPort || 8443,
|
||||
(reason) => this.connectionManager.initiateCleanupOnce(record, reason)
|
||||
(reason) => this.connectionManager.cleanupConnection(record, reason)
|
||||
);
|
||||
return;
|
||||
} else {
|
||||
@ -917,106 +983,6 @@ export class RouteConnectionHandler {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Setup improved error handling for the outgoing connection
|
||||
*/
|
||||
private setupOutgoingErrorHandler(
|
||||
connectionId: string,
|
||||
targetSocket: plugins.net.Socket,
|
||||
record: IConnectionRecord,
|
||||
socket: plugins.net.Socket,
|
||||
finalTargetHost: string,
|
||||
finalTargetPort: number
|
||||
): void {
|
||||
targetSocket.once('error', (err) => {
|
||||
// This handler runs only once during the initial connection phase
|
||||
const code = (err as any).code;
|
||||
logger.log('error',
|
||||
`Connection setup error for ${connectionId} to ${finalTargetHost}:${finalTargetPort}: ${err.message} (${code})`,
|
||||
{
|
||||
connectionId,
|
||||
targetHost: finalTargetHost,
|
||||
targetPort: finalTargetPort,
|
||||
errorMessage: err.message,
|
||||
errorCode: code,
|
||||
component: 'route-handler'
|
||||
}
|
||||
);
|
||||
|
||||
// Resume the incoming socket to prevent it from hanging
|
||||
socket.resume();
|
||||
|
||||
// Log specific error types for easier debugging
|
||||
if (code === 'ECONNREFUSED') {
|
||||
logger.log('error',
|
||||
`Connection ${connectionId}: Target ${finalTargetHost}:${finalTargetPort} refused connection. Check if the target service is running and listening on that port.`,
|
||||
{
|
||||
connectionId,
|
||||
targetHost: finalTargetHost,
|
||||
targetPort: finalTargetPort,
|
||||
recommendation: 'Check if the target service is running and listening on that port.',
|
||||
component: 'route-handler'
|
||||
}
|
||||
);
|
||||
} else if (code === 'ETIMEDOUT') {
|
||||
logger.log('error',
|
||||
`Connection ${connectionId} to ${finalTargetHost}:${finalTargetPort} timed out. Check network conditions, firewall rules, or if the target is too far away.`,
|
||||
{
|
||||
connectionId,
|
||||
targetHost: finalTargetHost,
|
||||
targetPort: finalTargetPort,
|
||||
recommendation: 'Check network conditions, firewall rules, or if the target is too far away.',
|
||||
component: 'route-handler'
|
||||
}
|
||||
);
|
||||
} else if (code === 'ECONNRESET') {
|
||||
logger.log('error',
|
||||
`Connection ${connectionId} to ${finalTargetHost}:${finalTargetPort} was reset. The target might have closed the connection abruptly.`,
|
||||
{
|
||||
connectionId,
|
||||
targetHost: finalTargetHost,
|
||||
targetPort: finalTargetPort,
|
||||
recommendation: 'The target might have closed the connection abruptly.',
|
||||
component: 'route-handler'
|
||||
}
|
||||
);
|
||||
} else if (code === 'EHOSTUNREACH') {
|
||||
logger.log('error',
|
||||
`Connection ${connectionId}: Host ${finalTargetHost} is unreachable. Check DNS settings, network routing, or firewall rules.`,
|
||||
{
|
||||
connectionId,
|
||||
targetHost: finalTargetHost,
|
||||
recommendation: 'Check DNS settings, network routing, or firewall rules.',
|
||||
component: 'route-handler'
|
||||
}
|
||||
);
|
||||
} else if (code === 'ENOTFOUND') {
|
||||
logger.log('error',
|
||||
`Connection ${connectionId}: DNS lookup failed for ${finalTargetHost}. Check your DNS settings or if the hostname is correct.`,
|
||||
{
|
||||
connectionId,
|
||||
targetHost: finalTargetHost,
|
||||
recommendation: 'Check your DNS settings or if the hostname is correct.',
|
||||
component: 'route-handler'
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
// Clear any existing error handler after connection phase
|
||||
targetSocket.removeAllListeners('error');
|
||||
|
||||
// Re-add the normal error handler for established connections
|
||||
targetSocket.on('error', this.connectionManager.handleError('outgoing', record));
|
||||
|
||||
if (record.outgoingTerminationReason === null) {
|
||||
record.outgoingTerminationReason = 'connection_failed';
|
||||
this.connectionManager.incrementTerminationStat('outgoing', 'connection_failed');
|
||||
}
|
||||
|
||||
// Clean up the connection
|
||||
this.connectionManager.initiateCleanupOnce(record, `connection_failed_${code}`);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets up a direct connection to the target
|
||||
@ -1074,13 +1040,21 @@ export class RouteConnectionHandler {
|
||||
}
|
||||
|
||||
// Create the target socket with immediate error handling
|
||||
let connectionEstablished = false;
|
||||
|
||||
const targetSocket = createSocketWithErrorHandler({
|
||||
port: finalTargetPort,
|
||||
host: finalTargetHost,
|
||||
onError: (error) => {
|
||||
// Connection failed - clean up everything immediately
|
||||
// Check if connection record is still valid (client might have disconnected)
|
||||
if (record.connectionClosed) {
|
||||
logger.log('debug', `Backend connection failed but client already disconnected for ${connectionId}`, {
|
||||
connectionId,
|
||||
errorCode: (error as any).code,
|
||||
component: 'route-handler'
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
logger.log('error',
|
||||
`Connection setup error for ${connectionId} to ${finalTargetHost}:${finalTargetPort}: ${error.message} (${(error as any).code})`,
|
||||
{
|
||||
@ -1108,10 +1082,12 @@ export class RouteConnectionHandler {
|
||||
}
|
||||
|
||||
// Resume the incoming socket to prevent it from hanging
|
||||
socket.resume();
|
||||
if (socket && !socket.destroyed) {
|
||||
socket.resume();
|
||||
}
|
||||
|
||||
// Clean up the incoming socket
|
||||
if (!socket.destroyed) {
|
||||
if (socket && !socket.destroyed) {
|
||||
socket.destroy();
|
||||
}
|
||||
|
||||
@ -1119,8 +1095,6 @@ export class RouteConnectionHandler {
|
||||
this.connectionManager.cleanupConnection(record, `connection_failed_${(error as any).code || 'unknown'}`);
|
||||
},
|
||||
onConnect: () => {
|
||||
connectionEstablished = true;
|
||||
|
||||
if (this.settings.enableDetailedLogging) {
|
||||
logger.log('info', `Connection ${connectionId} established to target ${finalTargetHost}:${finalTargetPort}`, {
|
||||
connectionId,
|
||||
@ -1154,7 +1128,7 @@ export class RouteConnectionHandler {
|
||||
error: err.message,
|
||||
component: 'route-handler'
|
||||
});
|
||||
return this.connectionManager.initiateCleanupOnce(record, 'write_error');
|
||||
return this.connectionManager.cleanupConnection(record, 'write_error');
|
||||
}
|
||||
});
|
||||
|
||||
@ -1168,7 +1142,7 @@ export class RouteConnectionHandler {
|
||||
socket,
|
||||
targetSocket,
|
||||
(reason) => {
|
||||
this.connectionManager.initiateCleanupOnce(record, reason);
|
||||
this.connectionManager.cleanupConnection(record, reason);
|
||||
}
|
||||
);
|
||||
|
||||
@ -1252,7 +1226,7 @@ export class RouteConnectionHandler {
|
||||
connectionId,
|
||||
serverName,
|
||||
connInfo,
|
||||
(_connectionId, reason) => this.connectionManager.initiateCleanupOnce(record, reason)
|
||||
(_connectionId, reason) => this.connectionManager.cleanupConnection(record, reason)
|
||||
);
|
||||
|
||||
// Store the handler in the connection record so we can remove it during cleanup
|
||||
@ -1277,7 +1251,7 @@ export class RouteConnectionHandler {
|
||||
remoteIP: record.remoteIP,
|
||||
component: 'route-handler'
|
||||
});
|
||||
this.connectionManager.initiateCleanupOnce(record, reason);
|
||||
this.connectionManager.cleanupConnection(record, reason);
|
||||
});
|
||||
|
||||
// Mark TLS handshake as complete for TLS connections
|
||||
@ -1287,7 +1261,7 @@ export class RouteConnectionHandler {
|
||||
}
|
||||
});
|
||||
|
||||
// Only set up basic properties - everything else happens in onConnect
|
||||
// Set outgoing socket immediately so it can be cleaned up if client disconnects
|
||||
record.outgoing = targetSocket;
|
||||
record.outgoingStartTime = Date.now();
|
||||
|
||||
@ -1348,7 +1322,7 @@ export class RouteConnectionHandler {
|
||||
record.incomingTerminationReason = 'timeout';
|
||||
this.connectionManager.incrementTerminationStat('incoming', 'timeout');
|
||||
}
|
||||
this.connectionManager.initiateCleanupOnce(record, 'timeout_incoming');
|
||||
this.connectionManager.cleanupConnection(record, 'timeout_incoming');
|
||||
});
|
||||
|
||||
targetSocket.on('timeout', () => {
|
||||
@ -1375,7 +1349,7 @@ export class RouteConnectionHandler {
|
||||
record.outgoingTerminationReason = 'timeout';
|
||||
this.connectionManager.incrementTerminationStat('outgoing', 'timeout');
|
||||
}
|
||||
this.connectionManager.initiateCleanupOnce(record, 'timeout_outgoing');
|
||||
this.connectionManager.cleanupConnection(record, 'timeout_outgoing');
|
||||
});
|
||||
|
||||
// Apply socket timeouts
|
||||
|
Reference in New Issue
Block a user