In the event that the SendStream Stopped future is polled once and then dropped
before the remote side has stopped the stream, and instead the stream
has been finished, the waker registered in the connection stopped map is
never removed and is effectively "leaked" until the connection is
closed. This can lead to a large amount of memory being retained when a
connection is very long lived and many streams are used over the life of
that connection.
The previous logic assumed that a datagram must be available if the
future was notified, but this might not be the case if e.g. two
futures are concurrently waiting for datagrams, but only one datagram
has been received, since we use notify_waiters to wake all waiting
futures to be robust in the face of cancellation.
After a spurious wakeup, `Connection::open_uni` and `open_bi` would
switch to waiting for notifications about incoming streams, rather
than the intended notification about new stream ID flow control
budget. This was probably a copy-paste error from `poll_accept`.
When the application observes a stream finish or reset, we dispose of
its state via `StreamsState::stream_freed`, and queue a transmit of
stream flow control credit directly on the
`Connection`. `stream_freed` however also sets
`StreamsState::max_streams_dirty`, which is polled each time we
receive a packet to see whether a stream has become fully closed due
to an incoming frame. Together, these led to a MAX_STREAMS frame being
transmit both immediately, and following the next packet receipt. We
prevent that by unsetting the appropriate `max_streams_dirty` flags
when sending `MAX_STREAMS`.
Similar to existing logic handling finishing of a stopped stream in
StreamsState::received. The missing credit would be issued if a stream
is later closed through a different code path, but there is no
guarantee that would happen.
Reduces indirection and allows us to avoid cloning an `Arc` each time
one of the relevant events is waited on, and opens the door to
handwritten futures built on them.
Application datagrams may be too large to fit into a packet with ACKs
present. Therefore, if we have unsent ACKs, doing so may push a queued
application datagram into the next packet. If no other data is queued,
the first packet will then be ACK-only. Sending an ACK-only packet
without having received an ACK-eliciting packet following our last
ACK-only packet is illegal, and will trigger a debug assert.
By being slightly less enthusiastic about sending ACKs, we ensure that
any inadvertently generated ACK-only packet is legal and simplify our
logic slightly. This approach also seems to be popular with other
implementations, as indicated by ad-hoc feedback on the slack.
Benjamin Saunders