Most of the tests in `src/tests.rs` have the structure of a client
sending data to a server which in turn echoes the data back to the
client which reads it in full and compares it to what it initially sent.
Some of the tests use randomly generated configurations where the
`WindowUpdateMode` can be either `OnRead` or `OnReceive`. When using
`WindowUpdateMode::OnRead` the client can not first send all data to the
server and only then receive all data from the server. In such case the
server would eventually exhaust its window to the client, thus not being
able to forward bytes to the client, thus not accepting new bytes from
the client, thus the client would be blocked on sending and thus the
whole test would deadlock.
With this commit the client writes and reads concurrently and thus does
not run into a deadlock when executing with `WindowUpdateMode::OnRead`.
The current implementation always uses the configured receive window
for the remote. However, when not sending the initial window update,
this means that the remote stops sending before having used up its
credit which it assumes matches the default receive window as it has
not received an initial window update. Therefore, if `lazy_open` is
true, the default receive window must be used initially.
PR #73 introduced a lazy open option to defer sending the initial
frame when actual data is sent. If enabled and the initial data
size equals DEFAULT_CREDIT (i.e. 256 KiB), it is necessary to
immediately send a window update back when accepting such a
stream, but only if window update mode is `OnReceive`, otherwise
the update will be sent when the stream data is consumed.
With more options there are more ways how things can influence
each other, so I have changed some tests to work with randomly
generated configurations to test more combinations.
The field `Stream::pending` is already specific to window updates and
in fact of type `Option<Frame<WindowUpdate>>`. The method
`Stream::send_pending` also modifies the stream's receive window after
a successful delivery, so because of all this window update specific
logic, the rather generic name "pending" is hereby replaced with
"window_update" and "send_pending" with "send_pending_window_update".
While we reset the receive window in `Stream::send_pending` we forgot
to do so in case the initial send operation was successful. Because of
that we potentially issued multiple window updates to the remote which
would then have too much credit and their subsequent frames might even
be larger than the max. stream buffer size.
To fix this we now only use `Stream::send_pending` to send window
updates, rather than modifying the window in two more places.
If `WindowUpdateMode` is `OnRead` and a stream is in state `SendClosed`
when being dropped, we consider its buffer to determine if the remote
may be blocked and needs to receive a reset frame. This is a bit odd as
we could and should rather look at the stream's actual window size.
If it is 0 we know that the remote may be blocked and since the
stream is dropped no further window updates are forthcoming from our
side, hence a RST frame should be sent right away. If it is > 0 the
remote either has credit left or if it has not then the socket buffer
must contain more data and we will answer with a RST for any unknown
stream, so no action is necessary.
So why are we not looking a `window`? The reason is that we currently
update the window before the window update is enqueued and if `poll_read`
is not called again it may never be. To fix this we must ensure that the
value of window is only updated after the window update has been
enqueued properly so we know it will eventually be sent to the remote.
Once we have ensured that it is done correctly we may safely look at
`window` in `garbage_collect` and proceed as outlined above.
This PR adds an `is_closed` flag to `Connection` and if `true`,
`Connection::next_stream` returns immediately with `Ok(None)` to help
clients that keep calling this method after they received an error.
While the API contract of `Connection::next_stream` already states that
after `Ok(None)` or `Err(_)` have been returned no further invocations
of `next_stream` must happen, it is conceivable that clients still do
this (e.g. trying to go on after `Err(_)`) so it seems advisable to
handle this case in a more explicit way.
