It is not rare a large header can trigger such a CONTINUATION loop.
While the stream cannot recover from this issue, returning an error
instead of panicking makes the impact radius under better control.
This commit adds accessors to `client::Connection` and
`server::Connection` that return the current value of the
`SETTINGS_MAX_CONCURRENT_STREAMS` limit that has been sent by this peer
and acknowledged by the remote.
This is analogous to the `max_concurrent_send_streams` methods added in
PR #513. These accessors may be somewhat less useful than the ones for
the values negotiated by the remote, since users who care about this
limit are probably setting the builder parameter. However, it seems
worth having for completeness sake --- and it might be useful for
determining whether or not a configured concurrency limit has been acked
yet...
Part of #512
* refactor: Extract FramedWrite::buffer to a less generic function
Should cut out another 23 KiB (since I see it duplicated)
* refactor: Extract some duplicated code to a function
* refactor: Extract part of flush into a less generic function
* refactor: Extract a less generic part of connection
* refactor: Factor out a less generic part of Connection::poll2
* refactor: Extract a non-generic part of handshake2
* refactor: Don't duplicate Streams code on Peer (-3.5%)
The `P: Peer` parameter is rarely used and there is already a mechanism
for using it dynamically.
* refactor: Make recv_frame less generic (-2.3%)
* Move out part of Connection::poll
* refactor: Extract parts of Connection
* refactor: Extract a non-generic part of reclaim_frame
* comments
* Explicitly enable the `std` feature of indexmap
This crate depends on it anyway, and by explicitly turning it on we
avoid unreliable platform target detection that causes build failures on
some platforms.
* Bump indexmap to 1.5.2
This allows use of the `std` feature.
Co-authored-by: Taiki Endo <te316e89@gmail.com>
Co-authored-by: Taiki Endo <te316e89@gmail.com>
This PR adds accessors to `client::Connection` and `server::Connection`
that return the send stream concurrency limit on that connection, as
negotiated by the remote peer. This is part of issue #512.
I think we probably ought to expose similar accessors for other
settings, but I thought it was better to add each one in a separate,
focused PR.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
The `compare_and_swap` method on atomics is now deprecated in favor
of `compare_exchange`.
Since the author of #510 closed that PR, this is just #510 with rustfmt run.
I also removed an unnecessary trailing semicolon that the latest rust
compiler now complains about.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: Kornel <kornel@cloudflare.com>
Tokio's AsyncWrite trait once again has support for vectored writes in
Tokio 0.3.4 (see tokio-rs/tokio#3149.
This branch re-enables vectored writes in h2.
This change doesn't make all that big of a performance improvement in
Hyper's HTTP/2 benchmarks, but they use a BytesMut as the buffer.
With a buffer that turns into more IO vectors in bytes_vectored, there
might be a more noticeable performance improvement.
I spent a bit trying to refactor the flush logic to coalesce into fewer
writev calls with more buffers, but the current implementation seems
like about the best we're going to get without a bigger refactor. It's
basically the same as what h2 did previously, so it's probably fine.
We've adopted `tracing` for diagnostics, but currently, it is just being
used as a drop-in replacement for the `log` crate. Ideally, we would
want to start emitting more structured diagnostics, using `tracing`'s
`Span`s and structured key-value fields.
A lot of the logging in `h2` is already written in a style that imitates
the formatting of structured key-value logs, but as textual log
messages. Migrating the logs to structured `tracing` events therefore is
pretty easy to do. I've also started adding spans, mostly in the read
path.
Finally, I've updated the tests to use `tracing` rather than
`env_logger`. The tracing setup happens in a macro, so that a span for
each test with the test's name can be generated and entered. This will
make the test output easier to read if multiple tests are run
concurrently with `--nocapture`.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
`Streams.inner.ref` doesn't need to be incremented if we don't
return an `OpaqueStreamRef`.
This prevented the bug in `send_push_promise` from appearing
in the tests.
A connection can never have more than u32::MAX >> 1 streams, so we'll
never store more than that many in the store slab. Define the
`SlabIndex` as a `u32` to reduce the number of bytes moved around.
