request-response: Throttled can not count its receive budget properly.

[twittner]

In libp2p-request-response there is a Throttled type which internally maintains counters for inbound and outbound requests per peer and limits their number. For inbound requests, it decrements its receive budget by 1 when receiving a RequestResponseHandlerEvent::Request from its inner RequestResponse behaviour. This budget should increase again, either when the request has been answered via Throttled::send_response, or when the request processing has timed out. The problem is that Throttled can not perform this increment correctly.

If it increments on a RequestResponseHandlerEvent::InboundTimeout it can not know if this timeout corresponds to a previously seen RequestResponseHandlerEvent::Request, because the timeout can also happen when the remote has not finished sending the request which would not cause a Request event. If it does not increment here it relies on the application to eventually invoke Throttled::send_response which is not guaranteed.

If we would only increment the budget in Throttled::send_response if sending over the oneshot channel succeeds (indicating that the upgrade is still in progress because the oneshot receiver has not been dropped yet), and otherwise increment on a timeout event, we are subject to a race condition because even after a successful send, the next poll of the future may determine first that the time is up and cause a timeout which makes us increment twice.

The proper solution to this is to attach an ID to inbound requests and errors, such as timeouts. This would allow Throttled to maintain a set of current requests and increment only if the ID is a element of this set. Unfortunately the ProtocolsHandler trait does not provide a way to attach information to inbound upgrades like it does for outbound ones.

[romanb]

Resolved / Superseded by #1726.