From 6d4c3f153cf44cefead39b21191dfb33491bfa5a Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Sun, 23 Aug 2026 04:31:18 +0000 Subject: net: start every peer's stall clock stopped, not at whatever was in memory CNode's constructor never wrote nMsgWaitSince or nMsgWaitSize, so every peer began life with indeterminate values in the pair ProcessMessages uses to decide whether that peer has stopped sending, and the first partial message it read was measured against them. The guard restarts the clock only when nMsgWaitSince is 0 or the buffer has grown since the last look. Garbage in nMsgWaitSince is neither, so the very first look fell through to the timeout branch. A peer that merely split a message across two TCP segments -- header in one, body in the next, which is ordinary on any real link -- could be dropped as unreadable on first contact, before it had sent anything wrong and often before its version message. It was never reproducible either: what happened depended on what had last used that heap. The log reported the stall as roughly the age of the epoch: peer 127.0.0.1:8433 announced a 100 byte 'block' and has sent nothing more for 1786593923 s (0 of 100 payload bytes) -- dropping, this peer is unreadable Every node is running this. Introduced in 7f6e5ad, which added the pair to the class and to ProcessMessages but not to the constructor; the constructor went on initialising nIncompleteMessageStart, nIncompleteMessageSize and strIncompleteMessageCommand, the upstream members our implementation supersedes. 5490d66 removed their last reader from ProcessMessages, and nothing has read or written them since, so they go, with BTF_INCOMPLETE_MESSAGE_TIMEOUT_SECS -- decoy state in a networking struct is how this happened, and the merge ahead of us would carry it further. The initialiser list now follows declaration order, so a member missing from it is visible. The net-message selftest was asserting on those dead members too, which is why it could not have caught this: it read state no code path writes, and its stale-message case passed only when the garbage in nMsgWaitSince happened to look old. Rewired onto nMsgWaitSince / nMsgWaitSize / BTF_MSG_WAIT_MAX_SECS, and given the two cases it never covered -- a new peer's clock must start stopped, and a peer that is still delivering payload must not be dropped however old its clock is. Measured on the build host, 20 runs each: before, net-message failed 20/20 ("a fresh incomplete message does not disconnect immediately" failed 17/20 by itself). After, 20/20 pass. Rewired selftest against a reverted constructor fix: fails 20/20, deterministically, where the old suite caught it by luck or not at all. --- src/net.h | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) (limited to 'src/net.h') diff --git a/src/net.h b/src/net.h index 8b0245a..3c8c409 100644 --- a/src/net.h +++ b/src/net.h @@ -89,10 +89,9 @@ static const int BTF_HANDSHAKE_GRACE_SECS = 60; // A serialized object cannot be larger than MAX_SIZE, so no valid network // message needs the old 256 MB header allowance. Keep the wire cap aligned with -// what the deserializer can accept and bound how long one peer may sit on a -// partial payload after a complete header has arrived. +// what the deserializer can accept. How long a peer may sit on a partial +// payload after a complete header has arrived is BTF_MSG_WAIT_MAX_SECS, below. static const unsigned int MAX_PROTOCOL_MESSAGE_SIZE = MAX_SIZE; -static const int BTF_INCOMPLETE_MESSAGE_TIMEOUT_SECS = 2 * 60; // Ceiling on simultaneous connections. // @@ -656,9 +655,6 @@ public: int64 nLastSend; int64 nLastRecv; int64 nLastSendEmpty; - int64 nIncompleteMessageStart; - unsigned int nIncompleteMessageSize; - string strIncompleteMessageCommand; string strBtfAddr; string strBtfMeeting; @@ -699,11 +695,13 @@ public: nLastSend = 0; nLastRecv = 0; nLastSendEmpty = GetTime(); - nIncompleteMessageStart = 0; - nIncompleteMessageSize = 0; - strIncompleteMessageCommand.clear(); strBtfAddr.clear(); strBtfMeeting.clear(); + // Not waiting on anything yet. Left indeterminate, the first partial + // message this peer sends is measured against whatever was in this + // memory, and ProcessMessages drops it as unreadable on first contact. + nMsgWaitSince = 0; + nMsgWaitSize = 0; nStartingHeight = -1; // Push a version message -- cgit v1.2.3