Skip to content

Fix QUIC UDP recv busy-loop and recover dead stacks after backgrounding - #205

Open
panfilov-vladislav wants to merge 2 commits into
masterfrom
net-quic-recovery
Open

panfilov-vladislav wants to merge 2 commits into
masterfrom
net-quic-recovery

Conversation

@panfilov-vladislav

Copy link
Copy Markdown
Collaborator

Problem

When the OS tears down networking for a backgrounded app (e.g. iOS suspending
the network interface), the QUIC UDP socket starts returning fatal recv errors.
Two separate bugs made this unrecoverable:

  1. 100% CPU busy-loop. CustomUdpSocket::poll_recv() only special-cased
    WouldBlock from try_io(); any other error was silently discarded and the
    loop went straight back to poll_recv_ready(). Because try_io() only
    clears the reactor's readiness bit for WouldBlock, a real error left the
    socket marked ready, so the next iteration immediately re-entered recv(),
    hit the same error, and repeated — a synchronous spin inside a single
    poll() that pins a tokio worker at 100% CPU instead of ever yielding.

  2. Stack never recovers. Even once recv errors surface correctly, quinn's
    endpoint driver treats a fatal socket error as terminal and nothing
    recreates the socket. Inner::bind() only rebinds a stack when its address
    differs, but backgrounding kills socket liveness without changing the IP/port
    — so the dead stack (and with it DHT/peer connectivity) stayed dead until the
    app restarted, even after the network interface recovered.

Fix

  • net/src/quic.rs: propagate non-WouldBlock recv errors instead of
    discarding them, and track a died flag on CustomUdpSocket, exposed via
    Connector::is_dead().
  • lib/src/network/: surface that flag up through QuicStack → Stacks
    → Gateway::any_stack_is_dead(), and make Inner::bind() force a rebind when
    any current stack is dead, regardless of whether the bind address changed.

Vlad Panfilov added 2 commits November 3, 2026 21:08
poll_recv() only special-cased WouldBlock from try_io(); any other
error (e.g. the network interface going down, which happens when iOS
suspends networking for a backgrounded app) was silently discarded
and the loop went straight back to poll_recv_ready(). Since try_io()
only clears the reactor's readiness bit for WouldBlock, a real error
leaves the socket marked ready, so the next iteration immediately
re-entered recv(), hit the same error, and repeated - a synchronous
spin inside a single poll() call that pins a tokio worker thread at
100% CPU instead of ever yielding back to the executor.

Propagate non-WouldBlock errors instead so the caller can handle or
recreate the connection.
poll_recv() now correctly surfaces a fatal UDP error instead of busy-looping
(previous commit), but quinn's endpoint driver treats that as terminal and
nothing ever recreates the socket - the whole QUIC stack (and with it, DHT/
peer connectivity) stays dead until the app restarts, even though the actual
network interface recovers (e.g. once the app leaves the background).

Inner::bind() only rebinds a stack when its address differs from before,
which never happens here (backgrounding doesn't change the IP/port, just
kills the socket's liveness), so a stack that died was never getting
recreated even when something called bind() again with the same addresses.

Track a `died` flag on CustomUdpSocket, set it when poll_recv hits a fatal
(non-WouldBlock) error, and surface it through Connector/QuicStack/Gateway
so Inner::bind() also rebinds when any current stack is dead, regardless of
whether the address changed.
madadam
madadam previously approved these changes Nov 5, 2026
@madadam
madadam dismissed their stale review November 5, 2026 08:45

Found potential problems after I approved.

@madadam madadam left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a potential race condition here. Are we sure the is_dead flag is always going to be set before we try to rebind the network? The way it's implemented in the PR might still work in practice but only accidentally due to timing: we are continuously receiving on the socket so is_dead is likely to get set before the app gets notified about being resumed, but I'm not sure we can 100% rely on this.

One (incomplete) idea how to fix this: instead of tracking the is_dead flag, we can "probe" the socket before trying to rebind it. If the probe tells us the socket is dead, we rebind, otherwise we don't. I'm not sure how to implement such "probe" though...

@madadam madadam left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it would be great if this had tests. The tricky part is how to simulate the app going to the background and back (or alternatively, triggering the socket death programmatically).

@inetic

inetic commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

I believe this is a problem

https://github.com/equalitie/ouisync/pull/205/changes#diff-e496489be65d9ddf47a95c72ba5147733884a0cdd452a4704c0035c8e3c48c77R424

Previously we just ignored every error, which for the background use case was a problem, now we allow any error except WouldBlock to mark the socket as dead. But there are multiple other errors that may arrive on a UDP socket which are not fatal. So a better approach would be to make it explicit which errors set dead to true.

@inetic

inetic commented Nov 5, 2026

Copy link
Copy Markdown
Member

Could you do the "is stack dead" check inside the any_stack_needs_rebind function instead having it as a separate condition here

https://github.com/equalitie/ouisync/pull/205/changes#diff-6e45d86e6b0d1458f778d42fdea5e74217aeaab6a0ff1278ca3e042c569be2e9R706-R707

This is a smaller one, but if you're going to do more changes to the code it would be nice to include it.

@madadam

madadam commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

There might be a better way to do all this: The whole logic of detecting socket errors and rebinding it can be fully encapsulated in the CustomUdpSocket struct. This would mean none of the layers above it would have to change (e.g., the app wouldn't need to explicitly rebind the network on resume). This approach would also make it easier to test.

This branch has not been deployed

No deployments
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.

3 participants