Fix get_data livelock in protocol_transaction_out_106

[echennells]

Summary

A block-only get_data (no tx items) sent to a synced node makes handle_receive_get_data spin a worker thread indefinitely.

Mechanism

The get_data subscription is an unsubscriber<get_data::cptr> over a std::list; notify iterates if (!(*it)(...)) it = queue_.erase(it);. handle_receive_get_data returns false (drop), and during that same call send_transaction's completion branch resubscribes (SUBSCRIBE_CHANNEL -> push_back) into the list being iterated. erase returns the just-pushed node, so the same get_data is re-delivered to the handler forever. This is the existing // BUGBUG: registration race. site. Reachable on a default node (enable_relay defaults true, bip37): one block-only get_data wedges a worker thread and N peers exhaust the pool.

Fix (revised per review)

The earlier return true was wrong: it left the subscription open, allowing unbounded concurrent send_transaction walks (memory-exhaustion DoS). The subscription drop is intentional backpressure and is kept. This revision changes one thing -- the initial send is wrapped in POST, so send_transaction runs on a later strand turn, outside notify. return false and the resubscribe are unchanged, so drop-while-processing backpressure is preserved verbatim; the completion resubscribe simply no longer push_backs into the list notify is iterating.

On "resubscribe ... without gapping the strand after the last send": the deferral is one strand turn. A request fed following receipt of the last response arrives a network round-trip later, long after the resubscribe has run, so it is not dropped. A request already pipelined ahead of the response is ordered after the resubscribe by strand FIFO (verified below).

Verification (testnet3, current master)

• Unpatched: trigger -> one thread pegged 75-84% CPU, backtrace in handle_receive_get_data.
• Patched: same trigger -> 0% CPU, block still served.
• Backpressure under flood: 2,000 overlapping get_data x 100 tx -> patched stays memory-flat (bounded, drops overlap); a stay-subscribed return true build serves all 200k and grows RssAnon ~8x.
• Resubscribe gap: instrumented strand-sequence log shows strict DISPATCH,RESUB ordering, zero drops under a request -> response -> request pipelined probe -- the one-turn window is not enterable.

Alternative

If you'd prefer these out-protocols converge on protocol_block_out_106 -- stay subscribed with a single-in-flight guard rather than desubscribe/resubscribe, eliminating the gap entirely -- that is a ~4-line variant I can substitute. Kept this minimal deliberately.

Note: the underlying unsubscriber::notify

notify will re-enter any handler that resubscribes during its own notification, silently and unbounded; protocol_filter_out_70015 makes the same mistake independently (#1071). Worth considering whether notify should assert against a mid-notify resubscription, so future misuse fails fast instead of hanging a thread. Raised as a question, not addressed here.

[evoskuil]

The subscription drop is the mechanism that prevents handling additional messages of the same protocol while one is already being processed. The intent is to resubscribe once the message is fully handled, and without gapping the strand after the last send. The ensures that the caller may feed another request following receipt of the last request response without it getting dropped in a resubscribe gap.

Removing this behavior may fix the observed behavior, but unless the message is handled synchronously (which requests for multiples will not be, leaving the subscription open is a DoS bug.

[echennells]

Revised. The drop is the backpressure; the earlier return true removed it (unbounded concurrent walks = memory DoS). This keeps return false and the resubscribe, and only POSTs the initial send so the completion resubscribe runs outside notify instead of push_backing into the list it's iterating.

On "without gapping the strand": the gap is one strand turn -- a request following the last response arrives a round-trip later, a pipelined one is ordered after the resubscribe by strand FIFO (instrumented: strict DISPATCH,RESUB, zero drops); flood test confirms memory stays bounded.

An alternative that removes the gap entirely (converge on block_out_106) is in the description.

[evoskuil]

Yes, this looks right. The POST is to the strand, so the async loop starts iteration on the strand, with the subscription having been already dropped at by the time the loop starts. The loop sends from the strand (as required) and handles each send by sending another or resubscribing. Since the handler is queued immediately upon socket send, a request having been received by the peer as a response to the final send cannot be missed. And a request initiated by the peer prior to the send may be dropped (as intended).

The bug was that the resubscription could occur within the same function call as the subscription handler, in the case where no transaction was actually sent.