diff options
| author | bitflash-maintainer <noreply@bitflash.invalid> | 2026-08-23 04:31:18 +0000 |
|---|---|---|
| committer | bitflash-maintainer <noreply@bitflash.invalid> | 2026-08-23 04:37:36 +0000 |
| commit | 6d4c3f153cf44cefead39b21191dfb33491bfa5a (patch) | |
| tree | 767e6e9f5281a7da648acf0ac70a1f18175dad11 /src/rpc.cpp | |
| parent | c4c19bd894c8fc30c558e560e326aab538b23170 (diff) | |
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.
Diffstat (limited to 'src/rpc.cpp')
0 files changed, 0 insertions, 0 deletions