hjwsm1989 commented on code in PR #3690:
URL: https://github.com/apache/brpc/pull/3690#discussion_r4442181293
##########
src/brpc/policy/http2_rpc_protocol.cpp:
##########
@@ -1301,15 +1402,38 @@ H2StreamContext::H2StreamContext(bool
read_body_progressively)
#endif
, _stream_id(0)
, _stream_ended(false)
+ , _remote_ended(false)
+ , _response_committed(false)
+ , _rst_received(false)
+ , _rst_error(H2_NO_ERROR)
, _remote_window_left(0)
, _deferred_window_update(0)
, _correlation_id(INVALID_BTHREAD_ID.value)
, _decoded_header_list_size(0)
- , _rejected_error(H2_NO_ERROR) {
+ , _rejected_error(H2_NO_ERROR)
+ , _deferred_response_id(INVALID_BTHREAD_ID.value) {
header().set_version(2, 0);
-#ifndef NDEBUG
get_h2_bvars()->h2_stream_context_count << 1;
-#endif
+}
+
+int H2StreamContext::OnSocketFailed(bthread_id_t id, void* data, int
error_code) {
+ butil::intrusive_ptr<H2StreamContext> sctx(
+ static_cast<H2StreamContext*>(data), false);
+ sctx->_deferred_response_id.store(
+ INVALID_BTHREAD_ID.value, butil::memory_order_release);
+ SocketUniquePtr sock(std::move(sctx->_deferred_response_sock));
+ if (sock != NULL) {
+ sock->ReleaseIdleFailRef();
+ }
+ if (error_code != 0 && sctx->_conn_ctx != NULL) {
+ H2StreamContext* removed =
+ sctx->_conn_ctx->RemoveStreamAndDeferWU(sctx->stream_id());
+ if (removed != NULL) {
+ removed->RemoveOneRefForStage2();
Review Comment:
Thanks for catching this! Updated in `fe043067`:
1. **Distinguish notification registration failure from Socket failure in
`H2StreamContext::OnSocketFailed`**:
- When `error_code != 0` and `RemoveStreamAndDeferWU()` removes the
deferred stream while `sock != NULL && !sock->Failed()`, `OnSocketFailed()`
returns `error_code` (propagated through
`bthread_id_unlock(deferred_notify_id)` in
`H2UnsentResponse::AppendAndDestroySelf()`), whereas connection failures
(`sock->Failed()`) return `0`.
- Also handled `bthread_id_create()` failure in
`H2UnsentResponse::AppendAndDestroySelf()` by removing the stream, transferring
any deferred stream window update, releasing its stage-2 reference, and setting
`notify_reg_error`.
2. **Explicitly terminate the stream with `RST_STREAM(H2_INTERNAL_ERROR)`**:
- When `notify_reg_error != 0`, `AppendAndDestroySelf()` logs a
rate-limited warning, appends `RST_STREAM(H2_INTERNAL_ERROR)` for `_stream_id`
to `out`, and increments `h2_stream_reset_count`, so the peer fails the call
immediately instead of hanging until timeout while the connection stays healthy.
3. **Added unit tests**:
-
`H2UnsentMessage.deferred_response_notify_registration_failure_resets_stream`
in `test/brpc_h2_unsent_message_unittest.cpp`
-
`HttpTest.http2_deferred_response_notify_registration_failure_resets_stream` in
`test/brpc_http_rpc_protocol_unittest.cpp`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
