Skip to content

Outbound: Close the handler when removing it; VLESS Reverse Proxy: Don't start after closing - #7112

Open
earmark-halt wants to merge 1 commit into
XTLS:mainfrom
earmark-halt:outbound-close-removed-handler
Open

earmark-halt wants to merge 1 commit into
XTLS:mainfrom
earmark-halt:outbound-close-removed-handler

Conversation

@earmark-halt

@earmark-halt earmark-halt commented Oct 8, 2026 •

Copy link
Copy Markdown

outbound.Manager.RemoveHandler, which xray api rmo (HandlerService.RemoveOutbound) calls, deletes the handler from the map but never closes it, while inbound.Manager.RemoveHandler does. So whatever the removed outbound's Close() would release keeps running:

  • WireGuard creates its TUN in the constructor (a kernel TUN when available, otherwise a gVisor stack), and once used also has the device with its UDP socket and keepalives.
  • VLESS with reverse keeps its monitor task: the removed outbound keeps dialing the server every 2 s.
  • Same for MASQUE's tunnel. VLESS with testpre gets closed too, but its pre-connects only stop reliably with VLESS outbound: Back off failed testpre pre-connects; Don't wait for a pre-connection forever #7113: on main, Close() stops just the goroutines waiting to hand over a connection, not the ones redialing an unreachable server.

This shows up wherever outbounds are added and removed at runtime through the API, e.g. by panels.

Changes

  • RemoveHandler closes the handler it removed. It does so after releasing the manager lock, so a slow Close() (WireGuard) doesn't hold up Select()/GetHandler() for new connections. Removing an unknown tag still returns nil, and a removed handler is closed only once, since Manager.Close() no longer sees it.
  • VLESS starts the reverse 2 s after the outbound is created (a goroutine in New()), so an outbound removed within those 2 s would still start it after Close(). Reverse.Start() is now a no-op once Reverse.Close() has run.

The other callers of RemoveHandler (commander, metrics, reverse portal, VLESS inbound's reverse) remove their own internal outbounds, whose Close() is a no-op or idempotent.

Connections already in flight on a removed outbound behave as before unless its Close() tears them down (WireGuard, MASQUE). A reverse bridge that is already connected is left to end on its own; only new ones stop.

Reproduction (on 836a6fe)

Client: api with HandlerService behind a dokodemo-door inbound, freedom as the only configured outbound. Server: a plain TCP listener that only counts accepted connections.

VLESS reverse: the outbound is added with xray api ado and removed with xray api rmo. Afterwards xray api lso lists only the freedom outbound in all runs.

rmo after ado build accepts at rmo 3 s later 13 s later
7 s main 3 4 9
7 s this PR 3 3 3
0.5 s main 0 1 6
0.5 s this PR 0 0 0

WireGuard (noKernelTun: true): five outbounds, each added with ado and removed right away with rmo. Goroutines from the metrics listener's /debug/pprof/goroutine: main 11 → 72, this PR 11 → 12.

Tests

  • New TestRemoveHandlerClosesHandler (on main the handler is closed 0 times) and TestReverseNotStartedAfterClose (on main the monitor runs once).
  • go test ./... except testing/scenarios: the only failures also fail on main, tests that need resources/geo*.dat or network access.
  • go test ./testing/scenarios: pass.
  • go run ./infra/vformat/main.go -mode check -pwd ./: pass.

…n't start after closing

`xray api rmo` (HandlerService.RemoveOutbound) removed the outbound from
the manager without closing it, unlike inbounds. A removed WireGuard
outbound kept its TUN (and its device once used), a removed VLESS
outbound with `reverse` kept dialing the server every 2 s.

The handler is now closed once it is removed, outside the manager lock.

VLESS's reverse is started 2 s after the outbound is created, so an
outbound removed within that window still started it: Start() is now a
no-op after Close().
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.

1 participant