rendezvous: bound the wait for a dial, so a dead path is noticed

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,

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`.

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;
         }
 

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<std::string>& vDescOut);
 // Verify descriptors a peer sent and remember the good ones. Returns how many.