diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index 4feb973c..bf72a907 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -770,34 +770,73 @@ Http2Session *ClientHandler::select_http2_session( auto &http2_avail_freelist = shared_addr->http2_avail_freelist; if (http2_avail_freelist.size() >= min) { - auto session = http2_avail_freelist.head; - session->remove_from_freelist(); + for (auto session = http2_avail_freelist.head; session;) { + auto next = session->dlnext; - if (LOG_ENABLED(INFO)) { - CLOG(INFO, this) << "Use Http2Session " << session - << " from http2_avail_freelist"; - } + session->remove_from_freelist(); - if (session->max_concurrency_reached(1)) { - if (LOG_ENABLED(INFO)) { - CLOG(INFO, this) << "Maximum streams are reached for Http2Session(" - << session << ")."; + // session may be in graceful shutdown period now. + if (session->max_concurrency_reached(0)) { + if (LOG_ENABLED(INFO)) { + CLOG(INFO, this) + << "Maximum streams have been reached for Http2Session(" + << session << "). Skip it"; + } + + session = next; + + continue; } - } else { - session->add_to_avail_freelist(); + + if (LOG_ENABLED(INFO)) { + CLOG(INFO, this) << "Use Http2Session " << session + << " from http2_avail_freelist"; + } + + if (session->max_concurrency_reached(1)) { + if (LOG_ENABLED(INFO)) { + CLOG(INFO, this) << "Maximum streams are reached for Http2Session(" + << session << ")."; + } + } else { + session->add_to_avail_freelist(); + } + return session; } - return session; } DownstreamAddr *selected_addr = nullptr; for (auto &addr : shared_addr->addrs) { - if (addr.proto != PROTO_HTTP2 || (addr.http2_extra_freelist.size() == 0 && - addr.connect_blocker->blocked())) { + if (addr.in_avail || addr.proto != PROTO_HTTP2 || + (addr.http2_extra_freelist.size() == 0 && + addr.connect_blocker->blocked())) { continue; } - if (addr.in_avail) { + for (auto session = addr.http2_extra_freelist.head; session;) { + auto next = session->dlnext; + + // session may be in graceful shutdown period now. + if (session->max_concurrency_reached(0)) { + if (LOG_ENABLED(INFO)) { + CLOG(INFO, this) + << "Maximum streams have been reached for Http2Session(" + << session << "). Skip it"; + } + + session->remove_from_freelist(); + + session = next; + + continue; + } + + break; + } + + if (addr.http2_extra_freelist.size() == 0 && + addr.connect_blocker->blocked()) { continue; } diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index 50f54664..ae9ddc0f 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -358,7 +358,8 @@ void ConnectionHandler::graceful_shutdown_worker() { int ConnectionHandler::handle_connection(int fd, sockaddr *addr, int addrlen, const UpstreamAddr *faddr) { if (LOG_ENABLED(INFO)) { - LLOG(INFO, this) << "Accepted connection. fd=" << fd; + LLOG(INFO, this) << "Accepted connection from " + << util::numeric_name(addr, addrlen) << ", fd=" << fd; } if (get_config()->num_worker == 1) { diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index b30cab74..27cb8a18 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -137,7 +137,8 @@ Downstream::Downstream(Upstream *upstream, MemchunkPool *mcpool, chunked_request_(false), chunked_response_(false), expect_final_response_(false), - request_pending_(false) { + request_pending_(false), + request_header_sent_(false) { auto &timeoutconf = get_config()->http2.timeout; @@ -885,7 +886,7 @@ bool Downstream::accesslog_ready() const { return resp_.http_status > 0; } void Downstream::add_retry() { ++num_retry_; } -bool Downstream::no_more_retry() const { return num_retry_ > 5; } +bool Downstream::no_more_retry() const { return num_retry_ > 50; } void Downstream::set_request_downstream_host(const StringRef &host) { request_downstream_host_ = host; @@ -895,10 +896,13 @@ void Downstream::set_request_pending(bool f) { request_pending_ = f; } bool Downstream::get_request_pending() const { return request_pending_; } +void Downstream::set_request_header_sent(bool f) { request_header_sent_ = f; } + bool Downstream::request_submission_ready() const { return (request_state_ == Downstream::HEADER_COMPLETE || request_state_ == Downstream::MSG_COMPLETE) && - request_pending_ && response_state_ == Downstream::INITIAL; + (request_pending_ || !request_header_sent_) && + response_state_ == Downstream::INITIAL; } int Downstream::get_dispatch_state() const { return dispatch_state_; } diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index 8ae62847..d5514ea8 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -302,7 +302,11 @@ public: DefaultMemchunks *get_request_buf(); void set_request_pending(bool f); bool get_request_pending() const; + void set_request_header_sent(bool f); // Returns true if request is ready to be submitted to downstream. + // When sending pending request, get_request_pending() should be + // checked too because this function may return true when + // get_request_pending() returns false. bool request_submission_ready() const; // downstream response API @@ -471,6 +475,8 @@ private: // has not been established or should be checked before use; // currently used only with HTTP/2 connection. bool request_pending_; + // true if downstream request header is considered to be sent. + bool request_header_sent_; }; } // namespace shrpx diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index fb601f43..2a85f980 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -117,11 +117,11 @@ void Http2DownstreamConnection::detach_downstream(Downstream *downstream) { auto &resp = downstream_->response(); - if (submit_rst_stream(downstream) == 0) { - http2session_->signal_write(); - } - if (downstream_->get_downstream_stream_id() != -1) { + if (submit_rst_stream(downstream) == 0) { + http2session_->signal_write(); + } + http2session_->consume(downstream_->get_downstream_stream_id(), resp.unconsumed_body_length); diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index e8650c02..721fd5b7 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -264,25 +264,28 @@ int Http2Session::disconnect(bool hard) { state_ = DISCONNECTED; // When deleting Http2DownstreamConnection, it calls this object's - // remove_downstream_connection(). The multiple + // remove_downstream_connection(). The multiple // Http2DownstreamConnection objects belong to the same - // ClientHandler object. So first dump ClientHandler objects. + // ClientHandler object if upstream is h2 or SPDY. So be careful + // when you delete ClientHandler here. // // We allow creating new pending Http2DownstreamConnection with this // object. Upstream::on_downstream_reset() may add - // Http2DownstreamConnection. + // Http2DownstreamConnection to another Http2Session. - std::set handlers; - for (auto dc = dconns_.head; dc; dc = dc->dlnext) { - if (!dc->get_client_handler()) { - continue; - } - handlers.insert(dc->get_client_handler()); - } - for (auto h : handlers) { - if (h->get_upstream()->on_downstream_reset(hard) != 0) { - delete h; + for (auto dc = dconns_.head; dc;) { + auto next = dc->dlnext; + auto downstream = dc->get_downstream(); + auto upstream = downstream->get_upstream(); + + // Failure is allowed only for HTTP/1 upstream where upstream is + // not shared by multiple Downstreams. + if (upstream->on_downstream_reset(downstream, hard) != 0) { + delete upstream->get_client_handler(); } + + // dc was deleted + dc = next; } auto streams = std::move(streams_); @@ -734,46 +737,44 @@ int on_stream_close_callback(nghttp2_session *session, int32_t stream_id, auto dconn = sd->dconn; if (dconn) { auto downstream = dconn->get_downstream(); - if (downstream && downstream->get_downstream_stream_id() == stream_id) { - auto upstream = downstream->get_upstream(); + auto upstream = downstream->get_upstream(); - if (downstream->get_downstream_stream_id() % 2 == 0 && - downstream->get_request_state() == Downstream::INITIAL) { - // Downstream is canceled in backend before it is submitted in - // frontend session. + if (downstream->get_downstream_stream_id() % 2 == 0 && + downstream->get_request_state() == Downstream::INITIAL) { + // Downstream is canceled in backend before it is submitted in + // frontend session. - // This will avoid to send RST_STREAM to backend - downstream->set_response_state(Downstream::MSG_RESET); - upstream->cancel_premature_downstream(downstream); - } else { - if (downstream->get_upgraded() && - downstream->get_response_state() == Downstream::HEADER_COMPLETE) { - // For tunneled connection, we have to submit RST_STREAM to - // upstream *after* whole response body is sent. We just set - // MSG_COMPLETE here. Upstream will take care of that. - downstream->get_upstream()->on_downstream_body_complete(downstream); - downstream->set_response_state(Downstream::MSG_COMPLETE); - } else if (error_code == NGHTTP2_NO_ERROR) { - switch (downstream->get_response_state()) { - case Downstream::MSG_COMPLETE: - case Downstream::MSG_BAD_HEADER: - break; - default: - downstream->set_response_state(Downstream::MSG_RESET); - } - } else if (downstream->get_response_state() != - Downstream::MSG_BAD_HEADER) { + // This will avoid to send RST_STREAM to backend + downstream->set_response_state(Downstream::MSG_RESET); + upstream->cancel_premature_downstream(downstream); + } else { + if (downstream->get_upgraded() && + downstream->get_response_state() == Downstream::HEADER_COMPLETE) { + // For tunneled connection, we have to submit RST_STREAM to + // upstream *after* whole response body is sent. We just set + // MSG_COMPLETE here. Upstream will take care of that. + downstream->get_upstream()->on_downstream_body_complete(downstream); + downstream->set_response_state(Downstream::MSG_COMPLETE); + } else if (error_code == NGHTTP2_NO_ERROR) { + switch (downstream->get_response_state()) { + case Downstream::MSG_COMPLETE: + case Downstream::MSG_BAD_HEADER: + break; + default: downstream->set_response_state(Downstream::MSG_RESET); } - if (downstream->get_response_state() == Downstream::MSG_RESET && - downstream->get_response_rst_stream_error_code() == - NGHTTP2_NO_ERROR) { - downstream->set_response_rst_stream_error_code(error_code); - } - call_downstream_readcb(http2session, downstream); + } else if (downstream->get_response_state() != + Downstream::MSG_BAD_HEADER) { + downstream->set_response_state(Downstream::MSG_RESET); } - // dconn may be deleted + if (downstream->get_response_state() == Downstream::MSG_RESET && + downstream->get_response_rst_stream_error_code() == + NGHTTP2_NO_ERROR) { + downstream->set_response_rst_stream_error_code(error_code); + } + call_downstream_readcb(http2session, downstream); } + // dconn may be deleted } // The life time of StreamData ends here http2session->remove_stream_data(sd); @@ -803,9 +804,6 @@ int on_header_callback2(nghttp2_session *session, const nghttp2_frame *frame, return 0; } auto downstream = sd->dconn->get_downstream(); - if (!downstream) { - return 0; - } auto namebuf = nghttp2_rcbuf_get_buf(name); auto valuebuf = nghttp2_rcbuf_get_buf(value); @@ -915,10 +913,6 @@ int on_invalid_header_callback2(nghttp2_session *session, if (!sd || !sd->dconn) { return 0; } - auto downstream = sd->dconn->get_downstream(); - if (!downstream) { - return 0; - } int32_t stream_id; @@ -963,13 +957,6 @@ int on_begin_headers_callback(nghttp2_session *session, NGHTTP2_INTERNAL_ERROR); return 0; } - auto downstream = sd->dconn->get_downstream(); - if (!downstream || - downstream->get_downstream_stream_id() != frame->hd.stream_id) { - http2session->submit_rst_stream(frame->hd.stream_id, - NGHTTP2_INTERNAL_ERROR); - return 0; - } return 0; } case NGHTTP2_PUSH_PROMISE: { @@ -1130,11 +1117,6 @@ int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; } auto downstream = sd->dconn->get_downstream(); - if (!downstream || - downstream->get_downstream_stream_id() != frame->hd.stream_id) { - return 0; - } - auto upstream = downstream->get_upstream(); rv = upstream->on_downstream_body(downstream, nullptr, 0, true); if (rv != 0) { @@ -1169,10 +1151,6 @@ int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, } auto downstream = sd->dconn->get_downstream(); - if (!downstream) { - return 0; - } - if (frame->headers.cat == NGHTTP2_HCAT_RESPONSE || frame->headers.cat == NGHTTP2_HCAT_PUSH_RESPONSE) { rv = on_response_headers(http2session, downstream, session, frame); @@ -1218,13 +1196,9 @@ int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, nghttp2_session_get_stream_user_data(session, frame->hd.stream_id)); if (sd && sd->dconn) { auto downstream = sd->dconn->get_downstream(); - if (downstream && - downstream->get_downstream_stream_id() == frame->hd.stream_id) { - - downstream->set_response_rst_stream_error_code( - frame->rst_stream.error_code); - call_downstream_readcb(http2session, downstream); - } + downstream->set_response_rst_stream_error_code( + frame->rst_stream.error_code); + call_downstream_readcb(http2session, downstream); } return 0; } @@ -1326,9 +1300,7 @@ int on_data_chunk_recv_callback(nghttp2_session *session, uint8_t flags, return 0; } auto downstream = sd->dconn->get_downstream(); - if (!downstream || downstream->get_downstream_stream_id() != stream_id || - !downstream->expect_response_body()) { - + if (!downstream->expect_response_body()) { http2session->submit_rst_stream(stream_id, NGHTTP2_INTERNAL_ERROR); if (http2session->consume(stream_id, len) != 0) { @@ -1380,10 +1352,6 @@ int on_frame_send_callback(nghttp2_session *session, const nghttp2_frame *frame, auto http2session = static_cast(user_data); if (frame->hd.type == NGHTTP2_DATA || frame->hd.type == NGHTTP2_HEADERS) { - if ((frame->hd.flags & NGHTTP2_FLAG_END_STREAM) == 0) { - return 0; - } - auto sd = static_cast( nghttp2_session_get_stream_user_data(session, frame->hd.stream_id)); @@ -1393,8 +1361,12 @@ int on_frame_send_callback(nghttp2_session *session, const nghttp2_frame *frame, auto downstream = sd->dconn->get_downstream(); - if (!downstream || - downstream->get_downstream_stream_id() != frame->hd.stream_id) { + if (frame->hd.type == NGHTTP2_HEADERS && + frame->headers.cat == NGHTTP2_HCAT_REQUEST) { + downstream->set_request_header_sent(true); + } + + if ((frame->hd.flags & NGHTTP2_FLAG_END_STREAM) == 0) { return 0; } @@ -1419,29 +1391,42 @@ int on_frame_not_send_callback(nghttp2_session *session, if (LOG_ENABLED(INFO)) { SSLOG(INFO, http2session) << "Failed to send control frame type=" << static_cast(frame->hd.type) - << "lib_error_code=" << lib_error_code << ":" + << ", lib_error_code=" << lib_error_code << ": " << nghttp2_strerror(lib_error_code); } - if (frame->hd.type == NGHTTP2_HEADERS && - lib_error_code != NGHTTP2_ERR_STREAM_CLOSED && - lib_error_code != NGHTTP2_ERR_STREAM_CLOSING) { - // To avoid stream hanging around, flag Downstream::MSG_RESET. - auto sd = static_cast( - nghttp2_session_get_stream_user_data(session, frame->hd.stream_id)); - if (!sd) { - return 0; - } - if (!sd->dconn) { - return 0; - } - auto downstream = sd->dconn->get_downstream(); - if (!downstream || - downstream->get_downstream_stream_id() != frame->hd.stream_id) { - return 0; - } - downstream->set_response_state(Downstream::MSG_RESET); - call_downstream_readcb(http2session, downstream); + if (frame->hd.type != NGHTTP2_HEADERS || + lib_error_code == NGHTTP2_ERR_STREAM_CLOSED || + lib_error_code == NGHTTP2_ERR_STREAM_CLOSING) { + return 0; } + + auto sd = static_cast( + nghttp2_session_get_stream_user_data(session, frame->hd.stream_id)); + if (!sd) { + return 0; + } + if (!sd->dconn) { + return 0; + } + auto downstream = sd->dconn->get_downstream(); + + if (lib_error_code == NGHTTP2_ERR_START_STREAM_NOT_ALLOWED) { + // Migrate to another downstream connection. + auto upstream = downstream->get_upstream(); + + if (upstream->on_downstream_reset(downstream, false)) { + // This should be done for h1 upstream only. Deleting + // ClientHandler for h2 or SPDY upstream may lead to crash. + delete upstream->get_client_handler(); + } + + return 0; + } + + // To avoid stream hanging around, flag Downstream::MSG_RESET. + downstream->set_response_state(Downstream::MSG_RESET); + call_downstream_readcb(http2session, downstream); + return 0; } } // namespace @@ -1819,7 +1804,8 @@ void Http2Session::submit_pending_requests() { for (auto dconn = dconns_.head; dconn; dconn = dconn->dlnext) { auto downstream = dconn->get_downstream(); - if (!downstream || !downstream->request_submission_ready()) { + if (!downstream->get_request_pending() || + !downstream->request_submission_ready()) { continue; } @@ -2157,6 +2143,7 @@ int Http2Session::handle_downstream_push_promise_complete( auto upstream = promised_downstream->get_upstream(); promised_downstream->set_request_state(Downstream::MSG_COMPLETE); + promised_downstream->set_request_header_sent(true); if (upstream->on_downstream_push_promise_complete(downstream, promised_downstream) != 0) { diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 34881254..2d7e2876 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -1821,64 +1821,66 @@ void Http2Upstream::on_handler_delete() { } } -int Http2Upstream::on_downstream_reset(bool no_retry) { +int Http2Upstream::on_downstream_reset(Downstream *downstream, bool no_retry) { int rv; - for (auto downstream = downstream_queue_.get_downstreams(); downstream; - downstream = downstream->dlnext) { - if (downstream->get_dispatch_state() != Downstream::DISPATCH_ACTIVE) { - // This is error condition when we failed push_request_headers() - // in initiate_downstream(). Otherwise, we have - // Downstream::DISPATCH_ACTIVE state, or we did not set - // DownstreamConnection. - downstream->pop_downstream_connection(); - continue; - } - - if (!downstream->request_submission_ready()) { - // pushed stream is handled here - rst_stream(downstream, NGHTTP2_INTERNAL_ERROR); - downstream->pop_downstream_connection(); - continue; - } - + if (downstream->get_dispatch_state() != Downstream::DISPATCH_ACTIVE) { + // This is error condition when we failed push_request_headers() + // in initiate_downstream(). Otherwise, we have + // Downstream::DISPATCH_ACTIVE state, or we did not set + // DownstreamConnection. downstream->pop_downstream_connection(); + handler_->signal_write(); - downstream->add_retry(); - - std::unique_ptr dconn; - - if (no_retry || downstream->no_more_retry()) { - goto fail; - } - - // downstream connection is clean; we can retry with new - // downstream connection. - - dconn = handler_->get_downstream_connection(downstream); - if (!dconn) { - goto fail; - } - - rv = downstream->attach_downstream_connection(std::move(dconn)); - if (rv != 0) { - goto fail; - } - - rv = downstream->push_request_headers(); - if (rv != 0) { - goto fail; - } - - continue; - - fail: - if (on_downstream_abort_request(downstream, 503) != 0) { - return -1; - } - downstream->pop_downstream_connection(); + return 0; } + if (!downstream->request_submission_ready()) { + // pushed stream is handled here + rst_stream(downstream, NGHTTP2_INTERNAL_ERROR); + downstream->pop_downstream_connection(); + + handler_->signal_write(); + + return 0; + } + + downstream->pop_downstream_connection(); + + downstream->add_retry(); + + std::unique_ptr dconn; + + if (no_retry || downstream->no_more_retry()) { + goto fail; + } + + // downstream connection is clean; we can retry with new + // downstream connection. + + dconn = handler_->get_downstream_connection(downstream); + if (!dconn) { + goto fail; + } + + rv = downstream->attach_downstream_connection(std::move(dconn)); + if (rv != 0) { + goto fail; + } + + rv = downstream->push_request_headers(); + if (rv != 0) { + goto fail; + } + + return 0; + +fail: + if (on_downstream_abort_request(downstream, 503) != 0) { + rst_stream(downstream, NGHTTP2_INTERNAL_ERROR); + } + downstream->pop_downstream_connection(); + handler_->signal_write(); return 0; diff --git a/src/shrpx_http2_upstream.h b/src/shrpx_http2_upstream.h index f9c8fb82..84cdea0c 100644 --- a/src/shrpx_http2_upstream.h +++ b/src/shrpx_http2_upstream.h @@ -78,7 +78,7 @@ public: virtual int on_downstream_body_complete(Downstream *downstream); virtual void on_handler_delete(); - virtual int on_downstream_reset(bool no_retry); + virtual int on_downstream_reset(Downstream *downstream, bool no_retry); virtual int send_reply(Downstream *downstream, const uint8_t *body, size_t bodylen); virtual int initiate_push(Downstream *downstream, const StringRef &uri); diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 65e2cb37..9a454a21 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -344,6 +344,10 @@ int HttpDownstreamConnection::push_request_headers() { auto &httpconf = get_config()->http; + // Set request_sent to true because we write request into buffer + // here. + downstream_->set_request_header_sent(true); + // For HTTP/1.0 request, there is no authority in request. In that // case, we use backend server's host nonetheless. auto authority = StringRef(downstream_hostport); diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 6332b29f..0be62b62 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -1202,10 +1202,12 @@ void HttpsUpstream::on_handler_delete() { } } -int HttpsUpstream::on_downstream_reset(bool no_retry) { +int HttpsUpstream::on_downstream_reset(Downstream *downstream, bool no_retry) { int rv; std::unique_ptr dconn; + assert(downstream == downstream_.get()); + if (!downstream_->request_submission_ready()) { // Return error so that caller can delete handler return -1; diff --git a/src/shrpx_https_upstream.h b/src/shrpx_https_upstream.h index 0f9b8dd0..4544651c 100644 --- a/src/shrpx_https_upstream.h +++ b/src/shrpx_https_upstream.h @@ -73,7 +73,7 @@ public: virtual int on_downstream_body_complete(Downstream *downstream); virtual void on_handler_delete(); - virtual int on_downstream_reset(bool no_retry); + virtual int on_downstream_reset(Downstream *downstream, bool no_retry); virtual int send_reply(Downstream *downstream, const uint8_t *body, size_t bodylen); virtual int initiate_push(Downstream *downstream, const StringRef &uri); diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index 7b17c4c2..8077d426 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -1287,63 +1287,66 @@ void SpdyUpstream::on_handler_delete() { } } -int SpdyUpstream::on_downstream_reset(bool no_retry) { +int SpdyUpstream::on_downstream_reset(Downstream *downstream, bool no_retry) { int rv; - for (auto downstream = downstream_queue_.get_downstreams(); downstream; - downstream = downstream->dlnext) { - if (downstream->get_dispatch_state() != Downstream::DISPATCH_ACTIVE) { - // This is error condition when we failed push_request_headers() - // in initiate_downstream(). Otherwise, we have - // Downstream::DISPATCH_ACTIVE state, or we did not set - // DownstreamConnection. - downstream->pop_downstream_connection(); - continue; - } - - if (!downstream->request_submission_ready()) { - rst_stream(downstream, SPDYLAY_INTERNAL_ERROR); - downstream->pop_downstream_connection(); - continue; - } - + if (downstream->get_dispatch_state() != Downstream::DISPATCH_ACTIVE) { + // This is error condition when we failed push_request_headers() + // in initiate_downstream(). Otherwise, we have + // Downstream::DISPATCH_ACTIVE state, or we did not set + // DownstreamConnection. downstream->pop_downstream_connection(); - downstream->add_retry(); + handler_->signal_write(); - std::unique_ptr dconn; - - if (no_retry || downstream->no_more_retry()) { - goto fail; - } - - // downstream connection is clean; we can retry with new - // downstream connection. - - dconn = handler_->get_downstream_connection(downstream); - if (!dconn) { - goto fail; - } - - rv = downstream->attach_downstream_connection(std::move(dconn)); - if (rv != 0) { - goto fail; - } - - rv = downstream->push_request_headers(); - if (rv != 0) { - goto fail; - } - - continue; - - fail: - if (on_downstream_abort_request(downstream, 503) != 0) { - return -1; - } - downstream->pop_downstream_connection(); + return 0; } + if (!downstream->request_submission_ready()) { + rst_stream(downstream, SPDYLAY_INTERNAL_ERROR); + downstream->pop_downstream_connection(); + + handler_->signal_write(); + + return 0; + } + + downstream->pop_downstream_connection(); + + downstream->add_retry(); + + std::unique_ptr dconn; + + if (no_retry || downstream->no_more_retry()) { + goto fail; + } + + // downstream connection is clean; we can retry with new + // downstream connection. + + dconn = handler_->get_downstream_connection(downstream); + if (!dconn) { + goto fail; + } + + rv = downstream->attach_downstream_connection(std::move(dconn)); + if (rv != 0) { + goto fail; + } + + rv = downstream->push_request_headers(); + if (rv != 0) { + goto fail; + } + + return 0; + +fail: + if (on_downstream_abort_request(downstream, 503) != 0) { + rst_stream(downstream, SPDYLAY_INTERNAL_ERROR); + } + downstream->pop_downstream_connection(); + handler_->signal_write(); return 0; diff --git a/src/shrpx_spdy_upstream.h b/src/shrpx_spdy_upstream.h index 2d6bd244..af5f589a 100644 --- a/src/shrpx_spdy_upstream.h +++ b/src/shrpx_spdy_upstream.h @@ -72,7 +72,7 @@ public: virtual int on_downstream_body_complete(Downstream *downstream); virtual void on_handler_delete(); - virtual int on_downstream_reset(bool no_retry); + virtual int on_downstream_reset(Downstream *downstream, bool no_retry); virtual int send_reply(Downstream *downstream, const uint8_t *body, size_t bodylen); diff --git a/src/shrpx_upstream.h b/src/shrpx_upstream.h index f85bb776..bc013aa1 100644 --- a/src/shrpx_upstream.h +++ b/src/shrpx_upstream.h @@ -57,10 +57,11 @@ public: virtual int on_downstream_body_complete(Downstream *downstream) = 0; virtual void on_handler_delete() = 0; - // Called when downstream connection is reset. Currently this is - // only used by Http2Session. If |no_retry| is true, another - // connection attempt using new DownstreamConnection is not allowed. - virtual int on_downstream_reset(bool no_retry) = 0; + // Called when downstream connection for |downstream| is reset. + // Currently this is only used by Http2Session. If |no_retry| is + // true, another connection attempt using new DownstreamConnection + // is not allowed. + virtual int on_downstream_reset(Downstream *downstream, bool no_retry) = 0; virtual void pause_read(IOCtrlReason reason) = 0; virtual int resume_read(IOCtrlReason reason, Downstream *downstream,