2 files changed, 75 insertions, 22 deletions

diff --git a/src/main.cpp b/src/main.cpp
index 37a15c6..11eb11d 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -2827,11 +2827,13 @@ bool ProcessMessages(CNode* pfrom)
             if (LogAcceptsCategory("net")) printf("\n\nPROCESSMESSAGE SKIPPED %d BYTES\n\n", (int)(pstart - vRecv.begin()));
         vRecv.erase(vRecv.begin(), pstart);
 
-        // Read the header from a copy first. The old path consumed the header,
-        // noticed the payload was incomplete, inserted the header back at the
-        // front of the receive buffer, then slept inside the shared message
-        // thread. A peer could announce a large payload and drip bytes forever,
-        // making every pass pay for an O(n) insert and a 100 ms stall.
+        // Read the header from a copy first, and do not consume it until the
+        // whole body is here. The old path consumed the header, noticed the
+        // payload was incomplete, inserted the header back at the front of the
+        // receive buffer, then slept inside the shared message thread. A peer
+        // could announce a large payload and drip bytes forever, making every
+        // pass pay for an O(n) insert and a 100 ms stall -- a cheap way for one
+        // peer to slow the thread that serves all of them. (upstream #109)
         CMessageHeader hdr;
         CDataStream vHeader(vRecv.begin(), vRecv.begin() + sizeof(CMessageHeader),
                             vRecv.nType, vRecv.nVersion);
@@ -2841,7 +2843,10 @@ bool ProcessMessages(CNode* pfrom)
             if (LogAcceptsCategory("net")) printf("\n\nPROCESSMESSAGE: ERRORS IN HEADER %s\n\n\n", hdr.GetCommand().c_str());
             if (hdr.nMessageSize > MAX_PROTOCOL_MESSAGE_SIZE)
                 pfrom->fDisconnect = true;
-            pfrom->nIncompleteMessageStart = 0;
+            pfrom->nMsgWaitSince = 0;
+            pfrom->nMsgWaitSize  = 0;
+            // The header was read from a copy, so it is still in vRecv. Skip it
+            // explicitly or this loop never advances.
             vRecv.ignore(sizeof(CMessageHeader));
             continue;
         }
@@ -2852,30 +2857,63 @@ bool ProcessMessages(CNode* pfrom)
         unsigned int nPayloadAvailable = vRecv.size() - sizeof(CMessageHeader);
         if (nMessageSize > nPayloadAvailable)
         {
-            if (pfrom->nIncompleteMessageStart == 0 ||
-                pfrom->nIncompleteMessageSize != nMessageSize ||
-                pfrom->strIncompleteMessageCommand != strCommand)
+            // Wait for the rest of the message. The header stays in vRecv.
+            //
+            // This is the mechanism the original note asked for: "need a
+            // mechanism to give up waiting for overlong message size error".
+            // Without one the wait is unbounded, and unbounded here means
+            // permanent. The header stays at the front of vRecv and the loop
+            // breaks; if the body never completes, every later pass re-reads
+            // that same header and breaks in the same place. The peer is never
+            // parsed again -- no blocks, no inv, not even its version message,
+            // so it never reports a height either, which is why a wedged node
+            // has peers and GetPeerMedianHeight() still returns -1.
+            //
+            // Nothing rescues it. A header could claim up to 256 MB and still
+            // pass CMessageHeader::IsValid() -- now capped at
+            // MAX_PROTOCOL_MESSAGE_SIZE -- and any byte that arrives
+            // refreshes nLastRecv, so the inactivity timeout that drops a
+            // silent peer never fires on one that is merely unreadable. The
+            // socket stays ESTABLISHED and select() keeps offering it.
+            // Measured on a wedged pool: this branch ran 2014 times in the
+            // deaf window, against 607 in the healthy window before it.
+            if (LogAcceptsCategory("net")) printf("MESSAGE-BREAK 2\n");
+            //
+            // Time the STALL, not the wait. A big message on a slow link is
+            // still arriving, and dropping that peer would be a bug of our own
+            // making; what must be caught is a buffer that has stopped growing
+            // while we go on expecting more. So the clock restarts whenever
+            // even one more byte has landed since the last look.
+            //
+            // Upstream #109 times this differently: it restarts the clock only
+            // when the (size, command) pair changes, which drops a peer sending
+            // one big message slowly but honestly. Progress is the better test,
+            // so that half is kept as it was.
+            int64 nNow = GetTime();
+            if (pfrom->nMsgWaitSince == 0 || nPayloadAvailable > pfrom->nMsgWaitSize)
             {
-                pfrom->nIncompleteMessageStart = GetTime();
-                pfrom->nIncompleteMessageSize = nMessageSize;
-                pfrom->strIncompleteMessageCommand = strCommand;
+                pfrom->nMsgWaitSince = nNow;
+                pfrom->nMsgWaitSize  = nPayloadAvailable;
             }
-            else if (GetTime() - pfrom->nIncompleteMessageStart >
-                     BTF_INCOMPLETE_MESSAGE_TIMEOUT_SECS)
+            else if (nNow - pfrom->nMsgWaitSince > BTF_MSG_WAIT_MAX_SECS)
             {
-                LogPrint("net",
-                         "disconnecting %s: incomplete %s message, %u of %u bytes after %d seconds\n",
-                         pfrom->addr.ToString().c_str(), strCommand.c_str(),
-                         nPayloadAvailable, nMessageSize,
-                         BTF_INCOMPLETE_MESSAGE_TIMEOUT_SECS);
+                // Deliberately not behind a debug category. This is the node
+                // going deaf to a peer, which is the thing nobody could see.
+                printf("peer %s announced a %u byte '%s' and has sent nothing more "
+                       "for %d s (%d of %u payload bytes) -- dropping, this peer is unreadable\n",
+                       pfrom->addr.ToString().c_str(), nMessageSize,
+                       strCommand.c_str(), (int)(nNow - pfrom->nMsgWaitSince),
+                       (int)nPayloadAvailable, nMessageSize);
                 pfrom->fDisconnect = true;
             }
+            // No re-insert, no Sleep(): the header was never consumed, so there
+            // is nothing to put back, and sleeping here stalls every peer.
             break;
         }
-        pfrom->nIncompleteMessageStart = 0;
-        pfrom->nIncompleteMessageSize = 0;
-        pfrom->strIncompleteMessageCommand.clear();
         vRecv.ignore(sizeof(CMessageHeader));
+        // The body arrived, so this peer is readable again.
+        pfrom->nMsgWaitSince = 0;
+        pfrom->nMsgWaitSize  = 0;
 
         // Copy message to its own buffer
         CDataStream vMsg(vRecv.begin(), vRecv.begin() + nMessageSize, vRecv.nType, vRecv.nVersion);
diff --git a/src/net.h b/src/net.h
index 440550c..0317676 100644
--- a/src/net.h
+++ b/src/net.h
@@ -146,6 +146,16 @@ static const int64        BTF_NO_PEER_HEIGHT_MAX_SECS = 20 * 60;
 // minutes is far beyond that, and well inside BTF_RECV_TIMEOUT_SECS.
 static const int64        BTF_MSG_WAIT_MAX_SECS = 2 * 60;
 
+// Largest payload a peer may announce in a message header: MAX_PROTOCOL_MESSAGE_SIZE
+// is defined further up this file. This tree reached the same cap by the same
+// reasoning as upstream #109 -- a 256 MB allowance is eight times what the
+// deserializer will accept (serialize.h throws above MAX_SIZE), so the header
+// check admitted an allocation the parser would only reject later, after
+// BTF_MSG_WAIT_MAX_SECS of a stalled peer slot -- so only the note survives.
+//
+// Both definitions were textually clean and 59 lines apart, so the replay onto
+// upstream merged them without a conflict and only the compiler objected.
+
 // Send a ping after this long with nothing to say, so a peer running the
 // inactivity check above does not mistake a quiet node for a dead one. Must
 // stay well under BTF_RECV_TIMEOUT_SECS.
@@ -741,6 +751,11 @@ public:
     {
         if (hSocket != INVALID_SOCKET)
         {
+            // Route through BtfCloseSocket so the accounting sees this close.
+            // A raw closesocket() here was invisible to sockcount, which is
+            // why the leak numbers never added up, and leaving the descriptor
+            // set afterwards is how a number that now belongs to somebody else
+            // gets closed a second time. (#103)
             BtfCloseSocket(hSocket);
             hSocket = INVALID_SOCKET;
         }