Repository navigation
Rebind the local listener on accept errors instead of killing run() - #206
Open
panfilov-vladislav wants to merge 3 commits into
Open
panfilov-vladislav wants to merge 3 commits into
panfilov-vladislav wants to merge 3 commits into
Conversation
added 3 commits
November 3, 2026 21:24
Service::run()'s doc comment says it "never completes (unless it errors)", but its local_server.accept() arm propagated any accept error with `?`, which does exactly that: kills the entire service - all local AND remote connections, repository expiration, everything - not just local ones. The local listener's socket can be invalidated by the OS independently of anything the service did (e.g. iOS tearing down a backgrounded app's sockets), and nothing ever recreated it, so once that happened the backend was permanently dead until the app was restarted: the Dart client's own reconnect-with-backoff kept retrying forever against a service that had already stopped listening, which is what presented as file previews and network status hanging indefinitely after a background/foreground cycle. Rebind on an accept error instead, with capped exponential backoff. local_server becomes Option<LocalServer> (None only transiently during a rebind) so the old (dead) listener is actually dropped before attempting to bind the new one - binding to the same port while the old listener value is still alive fails with AddrInUse even when its socket is already unusable at the OS level, which the included regression test caught.
run() is documented as safe to cancel and restart, but rebind_local_server left `local_server` as None across its backoff awaits. Since the Service outlives the run() future, a cancel during a rebind left the Service with no listener, and the next run() panicked on the `expect`/`unwrap`. Store the resolved local endpoint in its own field and rebind at the top of run()'s loop whenever `local_server` is None (the accept-error arm now just drops the dead listener). A restart that finds no listener rebinds instead of panicking, and local_endpoint() no longer unwraps an Option. Adds a regression test that starts run() with a missing listener.
A reviewer noted rebind_local_server is awaited inline, so while it retries the run() select! is paused and remote accepts, connection cleanup and repo expiration don't progress. Rebinds succeed fast in practice and a persistent failure means the local listener is unusable anyway, so keep it inline for now; record the trade-off and how to make it concurrent if needed.
madadam
approved these changes
Nov 5, 2026
This branch has not been deployed
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.
Problem
Service::run()is documented to never complete unless it errors, but itslocal_server.accept()arm propagated any accept error with?— tearing downthe entire service (all local and remote connections, repo expiration,
everything), not just the local listener.
The local listener's socket can be invalidated by the OS independently of
anything the service does — e.g. iOS tearing down a backgrounded app's sockets.
Nothing recreated the listener, so once that happened the backend was
permanently dead until the app restarted. This is what presented as file
previews and network status hanging indefinitely after a background/foreground
cycle: the Dart client's reconnect-with-backoff kept retrying forever against a
service that had already stopped listening.
Fix
propagating it.
local_serverbecomesOption<LocalServer>(None onlytransiently during a rebind) so the dead listener is dropped before binding
the replacement — binding the same port while the old value is still alive
fails with
AddrInUseeven once its socket is unusable at the OS level (theregression test caught this).
run()is documented as safe to cancel and restart,but a cancel mid-rebind left the longer-lived
Servicewith no listener, andthe next
run()panicked on theexpect/unwrap. The resolved localendpoint now lives in its own field, and
run()rebinds at the top of itsloop whenever
local_serverisNone— a restart that finds no listenerrebinds instead of panicking, and
local_endpoint()no longer unwraps anOption. Added a regression test that startsrun()with a missing listener.