fix(asio): don't lose the read wakeup - #882

The remaining hole in the CAsioSocketImpl read handoff, after #871 closed the visibility one. Observed wedging a live amulegui/amuled pair five times in six hours, each ending in the client's 30-second watchdog reconnect.

m_eventPending was a plain bool, set from the ASIO thread and cleared on the main thread, tested and set in separate steps:

if (!m_eventPending) { m_eventPending = true; notify(); }   // ASIO thread
void EventProcessed() { m_eventPending = false; }           // main thread

The damaging interleaving is the ASIO thread seeing the latch set and skipping its post, while the main thread is about to clear it. The event it deferred to belongs to the previous delivery, so the bytes it just buffered end up with nothing left to announce them (c = buffered, p = read pending, e = event pending):

handleRead(10)[c=10 p=0 e=1]   bytes land, latch already set
postSkip(2)   [c=10 p=0 e=1]   post skipped
block(8)      [c=10 p=0 e=0]   main consumes that event, latch clear

Final state: kernel_unread=4669 event_pending=0 read_pending=0 buffered=10. No read armed, no event outstanding, data on both sides of the boundary, and nothing in the design that can recover it.

The fix

Both sides now work on an atomic. PostReadEvent uses an acq_rel exchange, which makes test-and-set a single step so exactly one caller wins the right to notify — a plain test-then-set lets both threads see it clear and post twice, or lets the ASIO thread skip on a latch the main thread is about to clear.

Less obviously, exchange(true) writes unconditionally, so even the skipping path releases everything written before it: the filled buffer and the cleared m_readPending. EventProcessed acquires that same value. Without the acquire on the clearing side there is no happens-before edge from the ASIO thread's writes to the reader, and the reader stays free to observe a stale m_readPending and block on data that has already arrived.

PostReadEvent also now checks the wrapper before taking the latch rather than after. With no wrapper nothing delivers a notification, and EventProcessed only runs off a delivered one, so claiming the latch there left it set for the life of the socket and made every later post skip — the same wedge reached from the other side. HandleRead gets there deliberately: it logs "wrapper gone" and carries on to post. Returning early also avoids the window a take-then-release would open, where a concurrent post on the other thread sees the latch set and drops itself.

Scope

CAsioSocketImpl is TCP-only and backs eD2k peers, server links, proxies and EC alike, so this is a hot path for every TCP socket rather than an EC-only one. UDP goes through CAsioUDPSocketImpl, which has neither flag, so Kad is untouched.

Testing

Throughput measured on a 30 GB LAN transfer over 2.5 GbE, patched daemon on both ends, against master under the same conditions. Median download rate over T+30…T+90 of a 90 s ramp:

Like for like (both warm) the two are identical: 123.3 vs 123.3. The first row is the same branch measured on a cold seeder cache and is listed to show why a single pair is not enough — run-to-run spread on an unchanged build has been measured at 13 MB/s on this rig, so a lone −5% reading says nothing.

Measured on 76a832781; the tip adds only a debug log line on the no-wrapper path, which expands to do {} while(false) outside CONFIG:DEBUG and so is absent from the Release binaries that were benchmarked.

The unit suite passes, but that is not evidence for this change — nothing in it exercises CAsioSocketImpl. Nor would a throughput run reproduce the fault, which needs a specific interleaving rather than load; the measurement is here to show the change costs nothing on a path every transfer takes, not to demonstrate the fix.

Not included

An earlier revision of this branch also re-armed a background read from the blocking branch of Read(), intended to reap EC connections that linger in socket_list because a socket with no read armed never sees the peer's FIN. It was dropped: it reloaded m_readPending after the branch condition, so a completion landing between the two loads would be discarded by StartBackgroundRead() zeroing m_readBufferContent — losing bytes already taken from the kernel, which desyncs EC framing rather than merely stalling. Its trigger state was also close to unreachable: HandleRead only clears m_readPending with bytes_transferred > 0, and both error paths return with it still set, so pending == false && content == 0 is essentially the pre-SetWrapSocket state alone. The lingering-socket problem is real and still open, but wants its own evidence — the error paths leaving m_readPending set look like the more direct cause.