Skip to content

Conversation

blink-so[bot]
Copy link

@blink-so blink-so bot commented Oct 7, 2025

Problem

The proxy was building up CPU and memory usage over time due to goroutine leaks in CONNECT connection handling. Three specific leaks were identified:

  1. streamRequestToTarget (line 684): Spawned goroutine for request body copying that never synchronized or cleaned up properly
  2. handleConnectStreaming (line 733): Spawned goroutine for bidirectional tunnel copy that could hang indefinitely
  3. Both functions had no synchronization between their bidirectional copy goroutines

Solution

  • Use errgroup for proper goroutine lifecycle management and synchronization
  • Close write side of connections after copy completes to properly signal EOF
  • Add context monitoring to force cleanup when one goroutine completes
  • Improved error handling to ignore expected EOF/ErrClosed errors (reduced log noise)

The errgroup ensures that:

  • All spawned goroutines are tracked and waited on before the function returns
  • When one direction closes (e.g., client disconnects), the context is canceled
  • The cleanup goroutine forces both connections closed, unblocking any hanging io.Copy
  • No goroutines are left running after the connection handling completes

Testing

✅ All existing tests pass
✅ Code compiles without errors

Changes

  • Added golang.org/x/sync/errgroup dependency
  • Modified streamRequestToTarget to use errgroup with 3 goroutines (request copy, response copy, cleanup monitor)
  • Modified handleConnectStreaming to use errgroup with 3 goroutines (client→target, target→client, cleanup monitor)
  • Both functions now properly clean up all goroutines before returning
  • Connections can remain open indefinitely for long-lived use cases

blink-so bot and others added 2 commits October 7, 2025 19:39
This fixes three goroutine leaks that were causing CPU and memory usage to
build up over time:

1. streamRequestToTarget: Orphaned bidirectional copy goroutines that never
   terminated properly when connections closed early

2. handleConnectStreaming: Similar issue with bidirectional tunnel goroutines
   that could hang indefinitely

Changes made:
- Use errgroup to properly manage and synchronize goroutine lifecycle
- Add connection deadlines (5 min) to prevent indefinite blocking on io.Copy
- Close write side of connections after copy completes to signal EOF
- Add context cancellation monitoring to force cleanup if needed
- Improved error handling to ignore expected EOF/ErrClosed errors

Co-authored-by: f0ssel <[email protected]>
Connections need to stay open indefinitely for valid use cases.
The errgroup synchronization alone is sufficient to prevent goroutine leaks.

Co-authored-by: f0ssel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0 participants