Repository navigation
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
Open
earmark-halt wants to merge 1 commit into
earmark-halt wants to merge 1 commit into
Conversation
…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().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
outbound.Manager.RemoveHandler, whichxray api rmo(HandlerService.RemoveOutbound) calls, deletes the handler from the map but never closes it, whileinbound.Manager.RemoveHandlerdoes. So whatever the removed outbound'sClose()would release keeps running:reversekeeps its monitor task: the removed outbound keeps dialing the server every 2 s.testpregets closed too, but its pre-connects only stop reliably with VLESS outbound: Back off failedtestprepre-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
RemoveHandlercloses the handler it removed. It does so after releasing the manager lock, so a slowClose()(WireGuard) doesn't hold upSelect()/GetHandler()for new connections. Removing an unknown tag still returns nil, and a removed handler is closed only once, sinceManager.Close()no longer sees it.New()), so an outbound removed within those 2 s would still start it afterClose().Reverse.Start()is now a no-op onceReverse.Close()has run.The other callers of
RemoveHandler(commander, metrics, reverse portal, VLESS inbound's reverse) remove their own internal outbounds, whoseClose()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:
apiwithHandlerServicebehind a dokodemo-door inbound,freedomas the only configured outbound. Server: a plain TCP listener that only counts accepted connections.VLESS
reverse: the outbound is added withxray api adoand removed withxray api rmo. Afterwardsxray api lsolists only thefreedomoutbound in all runs.rmoafteradormoWireGuard (
noKernelTun: true): five outbounds, each added withadoand removed right away withrmo. Goroutines from themetricslistener's/debug/pprof/goroutine: main 11 → 72, this PR 11 → 12.Tests
TestRemoveHandlerClosesHandler(on main the handler is closed 0 times) andTestReverseNotStartedAfterClose(on main the monitor runs once).go test ./...excepttesting/scenarios: the only failures also fail on main, tests that needresources/geo*.dator network access.go test ./testing/scenarios: pass.go run ./infra/vformat/main.go -mode check -pwd ./: pass.