Fix wake-from-sleep SIGSEGV in asio socket impls (#384) - #596

Summary

• Fixes the wake-from-sleep SIGSEGV reported in aMule crashes with segmentation fault on system clock jump (e.g. when computer awakens from sleep, or on dailight saving change). #384 (both the original 2024-11 trace from @dreirund and the 2026-05 reproduction from @danim7).
• Replaces the 1-second-timer guard in Destroy() with std::enable_shared_from_this lifetime, eliminating the use-after-free that the timer never reliably prevented.
• Applied symmetrically to CAsioSocketImpl (TCP), CAsioSocketServerImpl (TCP acceptor), and CAsioUDPSocketImpl (UDP) since they share the exact vulnerability pattern.
• Plus a follow-up NULL-guard in CAsioUDPSocketImpl::DispatchClose for the post-resume EADDRINUSE path that surfaced once the UAF was fixed.

Background — the bug

CAsioUDPSocketImpl (and the TCP and acceptor impls) install asio completion handlers that capture a raw [this] (or, before commit 402fa9790, a boost::_bi::value<CAsioUDPSocketImpl*>). On wake-from-sleep, the following race plays out:

1. Before suspend: an async_receive_from is in flight, its completion handler holds a raw pointer to the impl.
2. System suspends for ≥ 20 min. asio worker threads are frozen.
3. On wake, the OS-level UDP socket has gone down/up; CMuleUDPSocket (the upper layer) tears down and recreates the lib socket. The old CLibUDPSocket::Destroy() schedules a 1-second-timer that eventually fires HandleDestroy() → delete m_libSocket → ~CAsioUDPSocketImpl → delete m_socket.
4. But during suspend the wall clock jumped forward by minutes. The 1-second timer is already "ready to fire" the moment asio threads wake — alongside any pending I/O completions queued by the reactor.
5. asio picks an order to run the strand-bound handlers. Nothing guarantees HandleDestroy runs after the pending recv completion. When HandleDestroy wins, it deletes the impl. The next handler then runs HandleRead on freed memory, which calls StartBackgroundRead() → m_socket->async_receive_from(...). m_socket is read from freed memory, yields a garbage pointer, and the dereference inside io_object_impl::get_service SIGSEGVs.

Both reports in #384 show this exact signature — the only differences are the value of the garbage this pointer (different runs, different freed-memory contents) and whether the handler was bound via boost::bind (pre-402fa9790) or a [this] lambda. The underlying bug is unchanged across that refactor.

Root cause

The 1-second-timer guard works during normal operation because the strand processes pending completions before the timer expires. It cannot survive a real system-clock jump, by design — expires_after(1s) measured against steady_clock is unaffected by wall-clock jumps, but the queueing order on the strand on wake-up is undefined.

More fundamentally, the design tries to express "let pending callbacks drain" by inserting a sentinel delay rather than by holding a reference. Any "delay" approach is racy; only reference-counted lifetime is correct.

Fix

Replace the timer band-aid with std::enable_shared_from_this:

class CAsioUDPSocketImpl : public std::enable_shared_from_this<CAsioUDPSocketImpl> {
    void StartBackgroundRead() {
        auto self = shared_from_this();
        m_socket->async_receive_from(buffer(...), m_receiveEndpoint,
            bind_executor(m_strand, [self](const error_code& ec, std::size_t n) {
                self->HandleRead(ec, n);
            }));
    }
};

The self capture keeps the impl alive as long as any callback holds a ref. The wrapper (CLibUDPSocket) holds the impl via std::shared_ptr<>; when the wrapper dies, it nulls a std::atomic<CLibUDPSocket*> m_libSocket back-pointer and drops its ref. Callbacks that fire after the wrapper is gone see the null back-pointer and skip their CoreNotify_* branch; the impl itself destructs cleanly once the last callback releases its self ref.

Destroy() becomes a single strand-posted teardown: null the back-pointer, close the asio socket, and (TCP path) fire CoreNotify_LibSocketDestroy to delete the wrapper on the GUI thread. No timer, no race.

Follow-up: DispatchClose NULL-guard

The second commit fixes a pre-existing latent bug that surfaced once the UAF was fixed: on the second wake-from-sleep, CMuleUDPSocket tears down and recreates its UDP socket faster than the strand can free port 4672, so bind() fails with EADDRINUSE. CreateSocket() leaves m_socket as NULL on the bind failure. CMuleUDPSocket then sees !IsOk() and calls Close(), which posts DispatchClose to the strand, which calls m_socket->close() on a NULL m_socket → SIGSEGV.

The fix is a one-liner: DispatchClose no-ops when m_socket is NULL. The EADDRINUSE itself is recoverable — CMuleUDPSocket retries the socket creation. Reported by @danim7 testing the first commit on the branch.

Scope

• src/LibSocketAsio.cpp — three impl classes refactored: CAsioSocketImpl, CAsioSocketServerImpl, CAsioUDPSocketImpl. Each gets enable_shared_from_this, atomic m_libSocket/m_libSocketServer, OnWrapperGone(), and a simplified Destroy(). m_isDestroying + m_timer + HandleDestroy() are removed. Plus the DispatchClose NULL-guard.
• src/LibSocket.h — three wrapper classes (CLibSocket, CLibSocketServer, CLibUDPSocket) hold the impl via std::shared_ptr<> instead of raw pointer; LinkSocketImpl() signature changes to take a shared_ptr by value (movable).
• No changes outside LibSocketAsio.cpp and LibSocket.h. The 14 callback sites are updated mechanically: every [this] capture becomes [self = shared_from_this()].

TCP and the acceptor are fixed in the same PR because they share the exact vulnerability pattern. Leaving them on the old timer-guard model would create two different asio lifetime models in the same file — bad for future maintainers, and a latent crash risk (no reported TCP/acceptor crash today, but the race is the same).

Validation

• Build clean on macOS arm64 (Apple Clang) — amuled + amulegui + unit tests.
• Build clean on Linux x86_64 — amuled + amulegui + unit tests.
• 9/9 unit tests pass on both platforms.
• Linux smoke test: amuled starts, sockets bind (TCP ECServer, UDP server, UDP client, TCP ListenSocket), connects to live ed2k peer, SIGTERM shutdown clean ("aMule shutdown completed.").
• Wake-from-sleep first-cycle on @danim7's recipe: no SIGSEGV with commit 1's shared_from_this fix. The second-cycle revealed the DispatchClose NULL-deref now fixed in commit 2; @danim7 is currently re-testing the two-commit branch.

Closes #384.