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.
[Trailers without EOS](https://httpwg.org/specs/rfc7540.html#HttpSequence):
> An endpoint that receives a HEADERS frame without the END_STREAM flag set after receiving a final (non-informational) status code MUST treat the corresponding request or response as malformed (Section 8.1.2.6).
[Malformed messages](https://httpwg.org/specs/rfc7540.html#malformed):
> Malformed requests or responses that are detected MUST be treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR.
Currently, there are many cases where `h2` will fail a connection or
stream with a PROTOCOL_ERROR, without recording why the protocol error
occurred. Since protocol errors may result from a bug in `h2` or from a
misbehaving peer, it is important to be able to debug the cause of
protocol errors.
This branch adds a log line to almost all cases where a protocol error
occurs. I've tried to make the new log lines consistent with the
existing logging, and in some cases, changed existing log lines to make
them internally consistent with other log lines in that module. All
receive-side errors that would send a reset are now logged at the debug
level, using a formatting based on the format used in `framed_read`.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
- Removes incrementing counter, instead just using the StreamId as a
slab ABA guard.
- Adjusts Ptr::deref to use Store::index, as before it was skipping to
check the ABA guard.
- Rename fields and types to clarify it actually is an ABA guard.
- Improve panic message in case a dangling Ptr is accessed.
Previously, any streams that were dropped or closed while not having
consumed the inflight received window capacity would simply leak that
capacity for the connection. This could easily happen if a `RecvStream`
were dropped before fully consuming the data, and therefore a user would
have no idea how much capacity to release in the first place. This
resulted in stalled connections that would never have capacity again.
* Avoid locking when printing
It is not obvious that attempting to print the
value of a struct could cause a deadlock. To avoid
this, this patch does not lock the mutex when generating
a debug representation of the h2 struct.
* Use try_lock
I believe this was an oversight - a stream that is reset can still have some
capacity assigned to it (e.g. if said capacity was assigned in the same poll as
the reset), which should be redistributed.
Because `send_reset` called `recv_err`, which calls `reclaim_all_capacity`,
which eventually calls `transition(stream, ..)` -- all of which happens _before_
the RESET frame is enqueued -- it was possible for the stream to get unlinked
from the store (if there was any connection-level capacity to reassign). This
could then cause the stream to get "leaked" on drop/EOF since it would no longer
be iterated.
Fix this by delaying the call to `reclaim_all_capacity` _after_ enqueueing the
RESET frame.
A test demonstrating the issue is included.
* Prevent `pending_open` streams from being released.
This fixes a panic that would otherwise occur in some cases. A test
demonstrating said panic is included.
* Clear the pending_open queue together with everything else.
Problem:
Applications may want to access the underlying h2 stream ID for
diagnostics, etc. Stream IDs were not previously exposed in public APIs.
Solution:
Added a new public `share::StreamId` type, which has a more restricted
API than the internal `frame::StreamId` type. The public API types
`SendStream`, `RecvStream`, `ReleaseCapacity`,
`client::ResponseFuture`, and `server::SendResponse` now all have
`stream_id` methods which return the stream ID of the corresponding
stream.
Closes#289.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Because `self.pending` doesn't necessarily get cleaned up in a timely fashion -
rather, only when the user calls `poll_ready()` - it was possible for it to
refer to a stream that has already been closed. This would lead to a panic the
next time that `poll_ready()` was called.
Instead, use an `OpaqueStreamRef`, bumping the refcount.
A change to an existing test is included which demonstrates the issue.
Previously, monotonic stream IDs (spec 5.1.1) for push promises were not
enforced. This was due to push promises going through an entirely
separate code path than normally initiated streams.
This patch unifies the code path for initializing streams via both
HEADERS and PUSH_PROMISE. This is done by first calling `recv.open` in
both cases.
Closes#272
The `is_peer_reset()` check doesn't quite cover all the cases where we call
`clear_queue`, such as when we call `recv_err`. Instead of trying to make the
check more precise, let's gracefully handle spurious entries in the queue.
