From d45f5a51e4e6d57e255e63e86ab44a7e6fa231b0 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 17 Feb 2015 21:40:42 +0900 Subject: [PATCH] nghttpx: Fix request resubmit bug on HTTP/2 backend connection check --- src/shrpx_downstream.cc | 6 +++++- src/shrpx_downstream.h | 6 ++++++ src/shrpx_http2_downstream_connection.cc | 9 ++++++--- src/shrpx_http2_session.cc | 7 ++++--- src/shrpx_http2_upstream.cc | 4 ++-- src/shrpx_https_upstream.cc | 4 ++-- src/shrpx_spdy_upstream.cc | 4 ++-- 7 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 7622e1b2..1a65f680 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -122,7 +122,7 @@ Downstream::Downstream(Upstream *upstream, int32_t stream_id, int32_t priority) request_connection_close_(false), request_header_key_prev_(false), request_http2_expect_body_(false), chunked_response_(false), response_connection_close_(false), response_header_key_prev_(false), - expect_final_response_(false) { + expect_final_response_(false), request_pending_(false) { ev_timer_init(&upstream_rtimer_, &upstream_rtimeoutcb, 0., get_config()->stream_read_timeout); @@ -1064,4 +1064,8 @@ void Downstream::set_request_downstream_host(std::string host) { request_downstream_host_ = std::move(host); } +void Downstream::set_request_pending(bool f) { request_pending_ = f; } + +bool Downstream::get_request_pending() const { return request_pending_; } + } // namespace shrpx diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index 810155ec..63ee8c04 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -183,6 +183,8 @@ public: void set_request_state(int state); int get_request_state() const; DefaultMemchunks *get_request_buf(); + void set_request_pending(bool f); + bool get_request_pending() const; // downstream response API const Headers &get_response_headers() const; // Lower the response header field names and indexes response @@ -387,6 +389,10 @@ private: bool response_connection_close_; bool response_header_key_prev_; bool expect_final_response_; + // true if downstream request is pending because backend connection + // should be checked before use; currently used only with HTTP/2 + // connection. + bool request_pending_; }; } // namespace shrpx diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index 74bdcc8e..f580f77f 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -222,16 +222,19 @@ ssize_t http2_data_read_callback(nghttp2_session *session, int32_t stream_id, int Http2DownstreamConnection::push_request_headers() { int rv; + if (!downstream_) { + return 0; + } if (!http2session_->can_push_request()) { // The HTTP2 session to the backend has not been established or // connection is now being checked. This function will be called // again just after it is established. + downstream_->set_request_pending(true); http2session_->start_checking_connection(); return 0; } - if (!downstream_) { - return 0; - } + + downstream_->set_request_pending(false); const char *authority = nullptr, *host = nullptr; if (!get_config()->no_host_rewrite && !get_config()->http2_proxy && diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index eb870bb5..5152f0f1 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -113,11 +113,12 @@ void writecb(struct ev_loop *loop, ev_io *w, int revents) { auto conn = static_cast(w->data); auto http2session = static_cast(conn->data); http2session->clear_write_request(); - http2session->connection_alive(); rv = http2session->do_write(); if (rv != 0) { http2session->disconnect(http2session->should_hard_fail()); + return; } + http2session->connection_alive(); } } // namespace @@ -1480,8 +1481,8 @@ void Http2Session::connection_alive() { for (auto dconn : dconns_) { auto downstream = dconn->get_downstream(); if (!downstream || - (downstream->get_request_state() != Downstream::HEADER_COMPLETE && - downstream->get_request_state() != Downstream::MSG_COMPLETE) || + (downstream->get_request_state() != Downstream::INITIAL && + !downstream->get_request_pending()) || downstream->get_response_state() != Downstream::INITIAL) { continue; } diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 3564c6d3..8746b71e 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -1476,8 +1476,8 @@ int Http2Upstream::on_downstream_reset(bool no_retry) { for (auto &ent : downstream_queue_.get_active_downstreams()) { auto downstream = ent.second.get(); - if ((downstream->get_request_state() != Downstream::HEADER_COMPLETE && - downstream->get_request_state() != Downstream::MSG_COMPLETE) || + if ((downstream->get_request_state() != Downstream::INITIAL && + !downstream->get_request_pending()) || downstream->get_response_state() != Downstream::INITIAL) { rst_stream(downstream, NGHTTP2_INTERNAL_ERROR); downstream->pop_downstream_connection(); diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 1c799c7e..1a64a7de 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -839,8 +839,8 @@ void HttpsUpstream::on_handler_delete() { int HttpsUpstream::on_downstream_reset(bool no_retry) { int rv; - if ((downstream_->get_request_state() != Downstream::HEADER_COMPLETE && - downstream_->get_request_state() != Downstream::MSG_COMPLETE) || + if ((downstream_->get_request_state() != Downstream::INITIAL && + !downstream_->get_request_pending()) || downstream_->get_response_state() != Downstream::INITIAL) { // Return error so that caller can delete handler return -1; diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index 4e76f683..cf48f716 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -1041,8 +1041,8 @@ int SpdyUpstream::on_downstream_reset(bool no_retry) { for (auto &ent : downstream_queue_.get_active_downstreams()) { auto downstream = ent.second.get(); - if ((downstream->get_request_state() != Downstream::HEADER_COMPLETE && - downstream->get_request_state() != Downstream::MSG_COMPLETE) || + if ((downstream->get_request_state() != Downstream::INITIAL && + !downstream->get_request_pending()) || downstream->get_response_state() != Downstream::INITIAL) { rst_stream(downstream, SPDYLAY_INTERNAL_ERROR); downstream->pop_downstream_connection();