From db34011b507fdacbb4511d2ae0847e4b0a8b76de Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 12 Aug 2026 16:33:35 +0000 Subject: net: one dead descriptor must not cost every peer its turn From the first of two wedges captured on the pool. It is a different proximate fault from the second one, and this is the part that turns a single bad socket into a node-wide outage. A failed select() zeroes both descriptor sets, so no peer is serviced on that pass. That is correct as far as it goes. What was missing is that nothing stopped the offending descriptor coming straight back: - the invalid socket was only marked fDisconnect, and a node is not removed from vNodes until it has drained or DISCONNECT_DRAIN_SECS has elapsed -- and it cannot drain, because its socket is gone; - the select set was rebuilt from vNodes with no check, so the same dead descriptor went back in on the very next pass and failed select() again. Measured in the capture: 1,027 consecutive 'its socket is no longer valid' passes across 61 seconds -- the full drain timeout -- one bad socket, and for the whole of it the node read nothing from anybody. The peer that caused it had 57 bytes unsent, so it took the slow path out. Now the descriptor is forgotten as soon as getsockopt says it is not ours: select() succeeds again on the next pass and every other peer is served normally while the dead node ages out as before. Deliberately forgotten rather than closed. getsockopt has just said this descriptor is no longer ours; calling close() on it is precisely how an unrelated socket gets closed instead, which is the fault fixed in e742d0b. (cherry picked from commit f14c513b4e39d6203b73584f6dc01acbb5a9546e) --- src/net.cpp | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'src/net.cpp') diff --git a/src/net.cpp b/src/net.cpp index 964097b..77f9acf 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1471,11 +1471,29 @@ void ThreadSocketHandler2(void* parg) #else socklen_t nTypeLen = sizeof(nType); #endif - if (getsockopt(pnode->hSocket, SOL_SOCKET, SO_TYPE, (char*)&nType, &nTypeLen) != 0) + if (pnode->hSocket != INVALID_SOCKET && + getsockopt(pnode->hSocket, SOL_SOCKET, SO_TYPE, (char*)&nType, &nTypeLen) != 0) { printf("dropping node %s: its socket is no longer valid\n", pnode->addr.ToString().c_str()); pnode->fDisconnect = true; + + // Stop referring to it, or it goes back into the next + // select() set and fails it again. + // + // Marking fDisconnect is not enough: a node is only + // removed from vNodes once it has drained or once + // DISCONNECT_DRAIN_SECS has passed, so one dead + // descriptor kept failing select() for a full minute -- + // and a failed select() zeroes both sets above, which + // means NO peer is read on that pass. Measured on a + // wedged pool: 1,027 consecutive passes over 61 + // seconds, one bad socket, the entire node deaf. + // + // Not closed, only forgotten. getsockopt just told us + // this descriptor is not ours any more; closing it now + // is how an unrelated socket gets closed instead. + pnode->hSocket = INVALID_SOCKET; } } } @@ -1548,6 +1566,10 @@ void ThreadSocketHandler2(void* parg) { CheckForShutdown(0); SOCKET hSocket = pnode->hSocket; + // Forgotten above because it was already invalid. FD_ISSET on it + // is meaningless and recv()/send() would be operating on -1. + if (hSocket == INVALID_SOCKET) + continue; // // Receive -- cgit v1.2.3