abort uTP sockets when their UDP socket is torn down (fixes shutdown hang) + teardown regression test

[KSEGIT]

Problem

qBittorrent on macOS freezes permanently (force-quit required) after a sleep/wake cycle with a VPN active — qbittorrent/qBittorrent#24353, with spindump and live-process sample evidence. Investigation found two distinct defects behind the same user-visible hang, both triggered by a network interface stalling/vanishing across sleep/wake (e.g. a VPN utunN tunnel):

1. Blocking service sends: synchronous, untimed send_to() in LSD/UPnP/NAT-PMP parks the single io thread in a poll() writability wait that never returns (the spindump attached to the issue shows the io thread in on_lsd_announce → announce_lsd → lsd::announce_impl → poll across all samples). Now fixed upstream by 4138b27 ("fix blocking UDP socket in UPnP") and 38d78f0 ("non-blocking UDP socket in LSD and NAT-PMP") — this PR originally carried equivalent changes and has been rebased to drop them in favor of upstream's.
2. uTP teardown leak (this PR, second commit): even with non-blocking sends, session shutdown can hang forever on orphaned uTP sockets. Observed live on qBittorrent 5.2.2 / libtorrent 2.0.12.

Commit 1 — add regression test for bounded session teardown with LSD active

test/test_lsd_teardown.cpp (registered explicit in the Jamfile, matching its sibling test_lsd, and added to the Makefile dist list): with LSD enabled and an announcing torrent (driven via force_lsd_announce()), abort() + session_proxy destruction must complete within a bounded time; a watchdog (joined via a scope guard so an exception cannot unwind through a joinable thread) converts any future teardown deadlock into a fast, attributed failure instead of an indefinite test-binary hang.

Honest scope, also stated in the test: a healthy CI interface cannot simulate the stalled-interface trigger, and with no peer connections the test does not exercise the uTP teardown path below — it pins the bounded-teardown invariant. A deterministic simulation test for the uTP stall itself (building on simulation/test_utp.cpp's utp_small_kernel_send_buf backpressure pattern, plus a mid-transfer abort) is a natural follow-up; happy to add it here or separately if preferred.

Commit 2 — abort uTP sockets when their UDP socket is torn down

Observed live (qBittorrent 5.2.2 / libtorrent 2.0.12, macOS): quit initiated, resume data saved, then a permanent hang with the io thread alive — parked in kqueue_reactor::run → kevent and caught mid-session_impl::on_tick re-arming its 100 ms timer — zero open OS sockets, GUI blocked in the teardown join. sample captures attached below.

Root cause chain:

• utp_stream stores its completion handlers in plain std::function members (aux_/utp_stream.hpp); each handler owns a shared_ptr<peer_connection>, and the utp_stream is itself a member of that peer — a reference cycle (peer → socket → handler → peer) breakable only by utp_socket_impl::cancel_handlers().
• session_impl::abort() closes the UDP sockets without telling the uTP socket managers — utp_socket_manager::remove_udp_socket(), the function designed for exactly this, had no callers (the abort() code even carries a TODO: "closing the udp sockets here means that the uTP connections cannot be closed gracefully").
• A uTP socket that stalled on a send (EWOULDBLOCK — e.g. the vanished VPN interface; note the new non-blocking service sends make stalls like this more common, not less) loses its only wake-up when the UDP socket closes: session_impl::on_udp_writeable drops the notification on error (if (ec) return;), and should_delete() requires !m_stalled, so the stalled socket can never be deleted.
• on_tick() only initiates abort_stage2() (→ m_work.reset()) once m_undead_peers is empty and num_sockets() == 0. An orphaned uTP socket pins its peer in m_undead_peers forever and itself keeps num_sockets() > 0 — so the tick re-arms every 100 ms indefinitely, io_context::run() never returns, and session_proxy::~session_proxy()'s join() blocks forever.

Fix: close each listen socket's UDP socket through a new close_udp_listen_socket() helper — used by both session_impl::abort() and the runtime listen-socket removal path (the latter also stops the same references leaking while the session keeps running) — which notifies the uTP socket managers. remove_udp_socket() first flushes all stalled sockets via the existing writable() (the same unfiltered delivery on_udp_writeable() performs), so sockets stalled on the closed socket fail their sends and tear down (clearing m_stalled, making them deletable), then aborts every uTP socket bound to the closed UDP socket. Its must-be-closed-first precondition is documented at the declaration and definition. A ChangeLog entry is included.

Related recent work in this same teardown region: f6fdabb ("fix issue in uTP shutdown"), 507c600 ("fix use-after-free in uTP failure socket shutdown path") — those fix crashes; this fixes the hang.

Verification

• test_lsd, test_lsd_teardown (new), test_utp: pass on macOS (arm64) and Linux, on v2.0.12+patch and on RC_2_0 (on top of 4138b27/38d78f022). After rebasing this branch onto current RC_2_1, re-verified test_lsd_teardown and test_utp on macOS (arm64): both pass.
• On a patched qBittorrent 5.2.0 build on macOS: a 1 ms-interval sample of a quit shows the io thread actively executing abort()/teardown; the app exits in ~2.3 s through the previously-hanging ~session_proxy → join path (Dock-quit AppleEvent path: 5.9 s).
• Evidence attached below: two sample captures of the live uTP-variant hang (19:56 and 20:08 — the second catches the on_tick re-arm), sanitized timeline (app log + pmset sleep/wake correlation), and lsof output showing zero network sockets at hang time.

Related

qbittorrent/qBittorrent#24353 (primary report) · #4510, #2861, #1199 (session-destructor hangs) · qbittorrent/qBittorrent#23695, #23604, #13012, #19666 (same symptom) · downstream defense-in-depth: qbittorrent/qBittorrent#24649 (bounded default shutdown timeout).

🤖 Generated with Claude Code

https://claude.ai/code/session_01JbRhL98sdGs31KAXE9uaj7

[Copilot]

Pull request overview

This PR prevents the libtorrent network thread from being indefinitely blocked by LSD (Local Service Discovery) multicast announces by switching the announce send path from a synchronous send_to() to async_send_to(). It also adds a regression test intended to ensure session teardown remains bounded when LSD is enabled.

Changes:

• Convert lsd::announce_impl() to use async_send_to() and add an on_send() completion handler plus a per-interface m_send_pending guard.
• Cancel LSD retry timer and mark the interface disabled on send errors (while ignoring operation_aborted).
• Add a new teardown-boundedness test and register it in the Jamfile.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/lsd.cpp	Reworks LSD announce sending to be asynchronous and adds completion/error handling.
include/libtorrent/lsd.hpp	Adds on_send() declaration and tracks m_send_pending state.
test/test_lsd_teardown.cpp	Adds a regression test to ensure LSD-enabled session teardown completes within a bound.
test/Jamfile	Registers the new LSD teardown test.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[KSEGIT]

This code is no longer part of this PR — the branch was rebased and the LSD changes were dropped in favor of upstream 38d78f0/4138b27d5; the current diff does not touch src/lsd.cpp. On the substance (which predates both): the cast is unreachable-safe here. The format string is pure-ASCII %s/%d/%x, so snprintf cannot fail with a negative return for encoding reasons, and the maximum rendered length is bounded at 139 bytes (69 literal + 17 for "[ff15::efc0:988f]" + 5 for a 16-bit port + 40 for the hex infohash + 8 for the cookie, which is masked to 31 bits) against the 200-byte buffer, so truncation cannot occur either. A defensive if (msg_len < 0 || msg_len >= int(sizeof(msg))) return; would be reasonable hardening, but it belongs in a separate change against the upstream code.

[KSEGIT]

Implemented in the current revision: the test now drives the announce deterministically via th.force_lsd_announce() (test_lsd_teardown.cpp:131) and the settle sleep is reduced to 500ms.

[KSEGIT]

Reworked the patch based on further review (force-pushed 4786a6e21):

• Approach changed from async_send_to to non_blocking(true) + synchronous send, matching the existing udp_socket.cpp precedent. Smaller diff, no handler/buffer lifetime machinery, and a would_block send simply drops the datagram (these are best-effort discovery packets with retry at every layer).
• Extended the same fix to UPnP (SSDP M-SEARCH) and NAT-PMP sockets — they had the identical blocking-send pattern, so the sleep/wake wedge in Constantly seems to freeze on OSX 26.2 qbittorrent/qBittorrent#24353 could still trigger through them (would_block on the M-SEARCH sockets is treated as transient rather than disabling port forwarding).
• Test hardening: added a watchdog so a future teardown deadlock fails fast with attribution instead of hanging the test binary; the LSD announce is now driven deterministically via force_lsd_announce() (addresses the inline comment about the fixed sleep). The other inline comment concerned the snprintf length cast — that code is back to its upstream form, untouched by this PR.

Verification re-run after the rework: test_lsd + test_lsd_teardown pass on macOS (arm64) and Linux on this branch.

[Copilot]

🟢 Ready to approve

The changes are cohesive and correctly route UDP listen-socket teardown through uTP manager notification, with CI/build updates and a regression test to prevent future teardown hangs.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[KSEGIT]

Ready to merge