Fix data races across session/forwarder/registry, resolve POST/PUT hang, harden port allocation, add frontendURL env #157

Merged

bagas merged 6 commits from staging into main 2026-07-19 14:10:50 +07:00

Conversation 0 Commits 6 Files Changed 24

+596 -132

bagas commented 2026-07-19 14:03:30 +07:00

Owner

Copy Link

Copy Source

Summary

Consolidates race condition, concurrency, and lifecycle-safety fixes across the session, transport, forwarder, and port-registry packages, plus one small feature addition.

Bug Fixes

• Fix data races in SetChannel(), Channel(), and StartedAt() with mutex protection
• Fix waitForTCPIPForward() failing on non-tcpip-forward global requests
• Fix data races in forwarder and slug structs via sync.RWMutex
• Fix data race in HandleTCPForward() from outer-scope err variable capture
• Fix POST/PUT requests hanging in the HTTP handler
• Fix AddRange infinite loop when endPort == 65535 (uint16 wraparound)
• Fix Claim bypassing the allowed port range
• Fix Unassigned handing out the same port to concurrent callers
• Fix SetStatus silently creating entries for unknown ports
• Fix data race in registry.Update

Improvements

• Refactor Close() to release the lock before calling external code, avoiding deadlocks
• Make CLOSED a terminal state — SetChannel() / SetStatus() reject calls after Close()
• Add PortRegistry interface for interface segregation
• Set startedAt when the session starts running, not at construction

Feature

• Add frontendURL env var

Commits

Commit	Description
e8be839dac	Fix race conditions and improve lifecycle safety (#150)
6fd25387f7	fix(session): resolve race conditions (#151)
4118872ed8	fix(httphandler): post/put request hang (#154)
61a4791bcb	fix(port, session): harden port registry and fix concurrent allocation (#155)
f1fa9332ac	fix(registry): data race registry update (#156)
d0a5033964	feat: add frontendURL env

## Summary Consolidates race condition, concurrency, and lifecycle-safety fixes across the session, transport, forwarder, and port-registry packages, plus one small feature addition. ## Bug Fixes - Fix data races in `SetChannel()`, `Channel()`, and `StartedAt()` with mutex protection - Fix `waitForTCPIPForward()` failing on non-`tcpip-forward` global requests - Fix data races in `forwarder` and `slug` structs via `sync.RWMutex` - Fix data race in `HandleTCPForward()` from outer-scope `err` variable capture - Fix POST/PUT requests hanging in the HTTP handler - Fix `AddRange` infinite loop when `endPort == 65535` (uint16 wraparound) - Fix `Claim` bypassing the allowed port range - Fix `Unassigned` handing out the same port to concurrent callers - Fix `SetStatus` silently creating entries for unknown ports - Fix data race in `registry.Update` ## Improvements - Refactor `Close()` to release the lock before calling external code, avoiding deadlocks - Make `CLOSED` a terminal state — `SetChannel()` / `SetStatus()` reject calls after `Close()` - Add `PortRegistry` interface for interface segregation - Set `startedAt` when the session starts running, not at construction ## Feature - Add `frontendURL` env var ## Commits | Commit | Description | |---|---| | `e8be839dac` | Fix race conditions and improve lifecycle safety (#150) | | `6fd25387f7` | fix(session): resolve race conditions (#151) | | `4118872ed8` | fix(httphandler): post/put request hang (#154) | | `61a4791bcb` | fix(port, session): harden port registry and fix concurrent allocation (#155) | | `f1fa9332ac` | fix(registry): data race registry update (#156) | | `d0a5033964` | feat: add frontendURL env |

bagas added 6 commits 2026-07-19 14:03:30 +07:00

Fix race conditions and improve lifecycle safety (#150) ... e8be839dac

## Summary
Comprehensive fixes for race conditions, concurrency bugs, and code quality improvements in the lifecycle module and related test files.

## Changes

### Race Condition Fixes
- Fixed data race in `SetChannel()` and `Channel()` by adding mutex protection
- Fixed data race in `StartedAt()` by adding mutex protection
- Fixed test race conditions in `transport` and `interaction` packages where variables were shared between goroutines

### Concurrency Safety Improvements
- Refactored `Close()` to release mutex before calling external code, preventing potential deadlocks
- Made `SetChannel()` reject calls after `Close()` to prevent setting channel on torn-down lifecycle
- Made `SetStatus()` ignore calls after `Close()` to ensure CLOSED is a terminal state
- Added nil check to `SetChannel()` to prevent masking upstream bugs

### Code Quality Improvements
- Refactored `Close()` to extract helper methods: `closeChannel()`, `closeConnection()`, `cleanupRegistry()`, `cleanupForwarder()`
- Fixed `isClosedError()` to remove redundant string comparison
- Added `PortRegistry` interface for better interface segregation (removed dependency on full `port.Port`)
- Made `cleanupRegistry()` conditional on non-empty slug to avoid unnecessary registry calls
- Fixed `startedAt` to be set when session starts running instead of at object creation

Reviewed-on: #150
Co-authored-by: Bagas <bagas@fossy.my.id>
Co-committed-by: Bagas <bagas@fossy.my.id>

fix(session): resolve race conditions (#151) ... 6fd25387f7

## Summary

Comprehensive fixes for race conditions, concurrency bugs, and a critical session handling bug across the session package and its child packages.

## Changes

### Bug Fix: waitForTCPIPForward fails on non-tcpip-forward global requests

- Changed `waitForTCPIPForward()` from a single select to a for loop that drains non-tcpip-forward requests gracefully
- Non-tcpip-forward requests now receive a false reply and the function continues waiting for the actual forward request
- The 500ms timeout resets after each request to handle slow clients with multiple early requests
- Updated and added tests to cover the new behavior: Wrong_Request_Type_Then_Timeout, Multiple_Non-Forward_Requests_Then_Success, Timeout_After_Non-Forward_Requests

### Race Condition Fixes

- Added sync.RWMutex to the forwarder struct to protect concurrent field access
- Fixed data race in forwarder setters (`SetType()`, `SetForwardedPort()`, `SetListener()`) by adding `Lock()`/`Unlock()` protection
- Fixed data race in forwarder getters (`TunnelType()`, `ForwardedPort()`, `Listener()`) by adding `RLock()`/`RUnlock()` protection
- Added sync.RWMutex to the slug struct to protect concurrent field access
- Fixed data race in `slug.Set()` by adding `Lock()`/`Unlock()` protection
- Fixed data race in `slug.String()` by adding `RLock()`/`RUnlock()` protection

### Concurrency Safety Improvements

- Changed `OpenForwardedChannel()` to use `f.ForwardedPort()` getter instead of direct field access
- Fixed `Close()` in forwarder to use `f.Listener()` getter consistently instead of accessing `f.listener` directly
- Fixed data race in `HandleTCPForward()` where the goroutine running `tcpServer.Serve(listener)` wrote to the outer err variable from the enclosing function scope
- Changed err = `tcpServer.Serve(listener)` to `if err := tcpServer.Serve(listener)` so the goroutine uses a local variable

Reviewed-on: #151
Co-authored-by: Bagas <bagas@fossy.my.id>
Co-committed-by: Bagas <bagas@fossy.my.id>

fix(httphandler): post/put request hang (#154) ... 4118872ed8

#153

Reviewed-on: #154
Co-authored-by: Bagas <bagas@fossy.my.id>
Co-committed-by: Bagas <bagas@fossy.my.id>

fix(port, session): harden port registry and fix concurent allocation (#155) ... 61a4791bcb

Critical fixes to the port registry that prevented correct tunnel port allocation under concurrency and allowed out-of-range ports to be claimed.

Bug Fixes:
- Fixed AddRange infinite loop when endPort == 65535 (uint16 wraparound)
 -Fixed Claim bypassing the allowed range — now rejects out-of-range ports
- Fixed Unassigned handing out the same port to concurrent callers — now reserves atomically
- Fixed SetStatus silently creating entries for unknown ports — now returns an error

Reviewed-on: #155
Co-authored-by: Bagas <bagas@fossy.my.id>
Co-committed-by: Bagas <bagas@fossy.my.id>

fix(registry): data race registry update (#156) ... f1fa9332ac

Concurrent map read and map write crashing the whole server

Bug fixes:
- Fix data race in registry.Update
Reviewed-on: #156
Co-authored-by: Bagas <bagas@fossy.my.id>
Co-committed-by: Bagas <bagas@fossy.my.id>

feat: add frontendURL env d0a5033964

bagas changed title from Fix race conditions and improve lifecycle safety (#150) to Fix data races across session/forwarder/registry, resolve POST/PUT hang, harden port allocation, add frontendURL env 2026-07-19 14:06:58 +07:00

bagas merged commit 5857dec730 into main 2026-07-19 14:10:50 +07:00

bagas deleted branch staging 2026-07-19 14:10:50 +07:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

Renovate-Clanker bagas

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bagas/tunnel-please#157