From e7c5ec2d86232cf00cc2f47046cde401abf4e444 Mon Sep 17 00:00:00 2001 From: Bitflash-sh Date: Thu, 30 Jul 2026 13:03:01 -0300 Subject: [PATCH] rendezvous: bound the wait for a dial, so a dead path is noticed A node registers at a relay and waits to be dialled. The wait had no bound, and ConnectTo() clears the socket timeout on purpose -- a registration is idle by design and may sit for a long time. What that cannot survive is the path dying with neither end saying so, which is the ordinary case and not the exotic one: a NAT drops the idle mapping, a relay restarts, a route changes. No FIN arrives, nothing becomes readable, and recv() never returns. The thread stays alive and asleep, so ThreadBtfAccept never reaches the `continue` that would register again. The node is unreachable and every outward sign says it is fine. Caught on the bootstrap seed: 27 minutes without a block, eight behind the network, relay reachable the whole time, and a restart fixing it instantly. Nothing in the log marked the moment it went deaf. I tried TCP keepalive first and it does not work here. With probes every 5s and a 2-probe limit, a socket whose replies were being dropped was still ESTAB after four minutes with the probe counter stuck at zero. It is measured, not assumed, and that approach is not in this change. So the bound is ours to enforce, at the one place that knows what waiting means. RvServiceWaitPaired takes a timeout; expiring is not an error but "nobody came, go round again", which the loop above already handles. Cost is one reconnect per relay per five minutes. Verified by killing a registration's path silently -- dropping only the relay's replies, so no RST or FIN is ever seen. With a 60s bound for the test: rendezvous: no dial at 92.246.128.180:8434 within 60s (or it dropped us), re-registering Registrations went 1 -> 2 and the node carried on. Before this, the same treatment left it blocked past four minutes with no log line at all. The socket goes back to blocking once a dial lands, because from that point it is a data tunnel and a tunnel is allowed to sit quiet between messages. --- src/btfrv.cpp | 51 +++++++++++++++++++++++++++++++++++++++++++++++++-- src/btfrv.h | 7 ++++++- src/net.cpp | 13 +++++++++---- src/net.h | 8 ++++++++ 4 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/btfrv.cpp b/src/btfrv.cpp index 63e092e..7efd38b 100644 --- a/src/btfrv.cpp +++ b/src/btfrv.cpp @@ -258,10 +258,57 @@ RvSocket RvServiceRegister(const char* relay_host, unsigned short port, return (RvSocket)s; } -bool RvServiceWaitPaired(RvSocket s) +bool RvServiceWaitPaired(RvSocket s, int nTimeoutSecs) { + // A registration socket is idle on purpose -- we announce ourselves and + // wait, possibly for a long time, for someone to dial. ConnectTo() clears + // the socket timeout for exactly that reason, and with no timeout at all + // this recv() has no way to end when the path dies without either side + // saying so: a NAT drops the idle mapping, a relay restarts, a route + // changes. No FIN arrives, nothing is readable, and the call never + // returns. The thread stays alive and asleep, so ThreadBtfAccept never + // reaches the `continue` that would register again, and the node goes deaf + // while every outward sign says it is healthy. + // + // Seen in production on the bootstrap seed: 27 minutes without a block, + // eight behind the network, relay reachable throughout, fixed instantly by + // a restart. + // + // TCP keepalive is the obvious answer and it does not work here -- I + // measured it. With probes every 5s and a 2-probe limit, a socket whose + // replies were being dropped was still ESTAB after four minutes, the probe + // counter stuck at zero. So the timeout is ours to enforce, at the one + // place that knows what waiting means. + // + // Expiring is not a failure. It costs one reconnect per interval and + // returns the loop to a known state. + if (nTimeoutSecs > 0) + { +#ifdef _WIN32 + DWORD tv = (DWORD)nTimeoutSecs * 1000; + setsockopt((SOCKET)s, SOL_SOCKET, SO_RCVTIMEO, (const char*)&tv, sizeof(tv)); +#else + struct timeval tv; tv.tv_sec = nTimeoutSecs; tv.tv_usec = 0; + setsockopt((SOCKET)s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); +#endif + } + unsigned char paired = 0; - return ReadN((SOCKET)s, &paired, 1) && paired == 0x01; + bool fOk = ReadN((SOCKET)s, &paired, 1) && paired == 0x01; + + // Hand back a socket that blocks again: from here it is a data tunnel, and + // a tunnel legitimately sits quiet between messages. + if (fOk && nTimeoutSecs > 0) + { +#ifdef _WIN32 + DWORD tvOff = 0; + setsockopt((SOCKET)s, SOL_SOCKET, SO_RCVTIMEO, (const char*)&tvOff, sizeof(tvOff)); +#else + struct timeval tvOff; tvOff.tv_sec = 0; tvOff.tv_usec = 0; + setsockopt((SOCKET)s, SOL_SOCKET, SO_RCVTIMEO, &tvOff, sizeof(tvOff)); +#endif + } + return fOk; } RvSocket RvClientConnect(const char* relay_host, unsigned short port, diff --git a/src/btfrv.h b/src/btfrv.h index 9d6756e..9cd7476 100644 --- a/src/btfrv.h +++ b/src/btfrv.h @@ -49,7 +49,12 @@ RvSocket RvServiceRegister(const char* relay_host, unsigned short port, // Block on a registered service socket until a client is paired to us. Returns // false if the relay drops us or errors, in which case the caller should close // the socket and register again. -bool RvServiceWaitPaired(RvSocket s); +// Blocks until a client is paired with us. Give it a timeout, in seconds, and +// it returns false once that passes with nobody arriving -- which the caller +// should treat as "register again", not as an error. Zero means wait forever, +// which is what this used to do unconditionally and must not be used for a +// registration socket: see the note on the implementation. +bool RvServiceWaitPaired(RvSocket s, int nTimeoutSecs = 0); // ---- client side (the node dialing a .btf address) ---- // Connect through the relay to the service registered under `target_pubkey`. diff --git a/src/net.cpp b/src/net.cpp index 395092d..2581578 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -783,12 +783,17 @@ void ThreadBtfAccept(void* parg) LogPrint("net", "rendezvous: registered at %s, waiting for a dial\n", strMeeting.c_str()); - // Now block for someone to arrive. - if (!btf::RvServiceWaitPaired(rv)) + // Now wait for someone to arrive -- but not forever. This used to have + // no bound, and a registration whose path died quietly left the thread + // parked in recv() with nothing to wake it: the loop never came back + // here, the node stopped being reachable, and nothing in the log said + // so. Re-registering every few minutes when nobody has dialled costs + // one reconnect and removes the whole failure mode. + if (!btf::RvServiceWaitPaired(rv, BTF_RENDEZVOUS_WAIT_SECS)) { btf::RvClose(rv); - LogPrint("net", "rendezvous: %s dropped us before any dial, re-registering\n", - strMeeting.c_str()); + LogPrint("net", "rendezvous: no dial at %s within %ds (or it dropped us), " + "re-registering\n", strMeeting.c_str(), BTF_RENDEZVOUS_WAIT_SECS); continue; } diff --git a/src/net.h b/src/net.h index 364fbfb..4379d04 100644 --- a/src/net.h +++ b/src/net.h @@ -43,6 +43,14 @@ static const unsigned int MAX_PEX_DESCRIPTOR_BYTES = 1024; // connection is the intended traffic; anything faster is someone else's idea. static const int64 PEX_MIN_INTERVAL = 60; +// How long to sit registered at a rendezvous relay before giving up on this +// attempt and registering again. Bounded because a registration socket that +// goes quiet is indistinguishable, from inside recv(), from one whose path has +// died -- and the second kind never wakes up. Five minutes is short enough +// that a node is unreachable only briefly and long enough that the reconnect +// traffic is nothing: one per relay per five minutes. +static const int BTF_RENDEZVOUS_WAIT_SECS = 300; + // Descriptors to hand a peer: ours first, then peers that answered us. void BtfPexCollect(std::vector& vDescOut); // Verify descriptors a peer sent and remember the good ones. Returns how many.