You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ic...@apache.org on 2016/08/24 13:30:11 UTC
svn commit: r1757524 - in /httpd/httpd/trunk: CHANGES modules/http2/h2.h
modules/http2/h2_filter.c modules/http2/h2_session.c
Author: icing
Date: Wed Aug 24 13:30:11 2016
New Revision: 1757524
URL: http://svn.apache.org/viewvc?rev=1757524&view=rev
Log:
mod_http2: graceful handling of open streams during graceful shutdown
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/http2/h2.h
httpd/httpd/trunk/modules/http2/h2_filter.c
httpd/httpd/trunk/modules/http2/h2_session.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1757524&r1=1757523&r2=1757524&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Wed Aug 24 13:30:11 2016
@@ -1,6 +1,9 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) mod_http2: handling graceful shutdown gracefully, e.g. handling existing
+ streams to the end. [Stefan Eissing]
+
*) mod_cache: Use the actual URI path and query-string for identifying the
cached entity (key), such that rewrites are taken into account when
running afterwards (CacheQuickHandler off). PR 21935. [Yann Ylavic]
Modified: httpd/httpd/trunk/modules/http2/h2.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2.h?rev=1757524&r1=1757523&r2=1757524&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2.h (original)
+++ httpd/httpd/trunk/modules/http2/h2.h Wed Aug 24 13:30:11 2016
@@ -95,8 +95,6 @@ typedef enum {
H2_SESSION_ST_IDLE, /* nothing to write, expecting data inc */
H2_SESSION_ST_BUSY, /* read/write without stop */
H2_SESSION_ST_WAIT, /* waiting for tasks reporting back */
- H2_SESSION_ST_LOCAL_SHUTDOWN, /* we announced GOAWAY */
- H2_SESSION_ST_REMOTE_SHUTDOWN, /* client announced GOAWAY */
} h2_session_state;
typedef struct h2_session_props {
@@ -106,6 +104,7 @@ typedef struct h2_session_props {
apr_uint32_t emitted_max; /* the highest local stream id sent */
apr_uint32_t error; /* the last session error encountered */
unsigned int accepting : 1; /* if the session is accepting new streams */
+ unsigned int shutdown : 1; /* if the final GOAWAY has been sent */
} h2_session_props;
Modified: httpd/httpd/trunk/modules/http2/h2_filter.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_filter.c?rev=1757524&r1=1757523&r2=1757524&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_filter.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_filter.c Wed Aug 24 13:30:11 2016
@@ -367,9 +367,7 @@ static apr_status_t h2_status_stream_fil
add_peer_settings(bb, s, 0);
bbout(bb, " \"connFlowIn\": %d,\n", connFlowIn);
bbout(bb, " \"connFlowOut\": %d,\n", connFlowOut);
- bbout(bb, " \"sentGoAway\": %d,\n",
- (s->state == H2_SESSION_ST_LOCAL_SHUTDOWN
- || s->state == H2_SESSION_ST_DONE));
+ bbout(bb, " \"sentGoAway\": %d,\n", s->local.shutdown);
add_streams(bb, s, 0);
Modified: httpd/httpd/trunk/modules/http2/h2_session.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_session.c?rev=1757524&r1=1757523&r2=1757524&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/h2_session.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_session.c Wed Aug 24 13:30:11 2016
@@ -75,7 +75,6 @@ static apr_status_t h2_session_receive(v
const char *data, apr_size_t len,
apr_size_t *readlen);
-static int is_accepting_streams(h2_session *session);
static void dispatch_event(h2_session *session, h2_session_event_t ev,
int err, const char *msg);
@@ -285,11 +284,6 @@ static int on_data_chunk_recv_cb(nghttp2
int rv;
(void)flags;
- if (!is_accepting_streams(session)) {
- /* ignore */
- return 0;
- }
-
stream = get_stream(session, stream_id);
if (!stream) {
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, APLOGNO(03064)
@@ -398,11 +392,6 @@ static int on_header_cb(nghttp2_session
apr_status_t status;
(void)flags;
- if (!is_accepting_streams(session)) {
- /* just ignore */
- return 0;
- }
-
stream = get_stream(session, frame->hd.stream_id);
if (!stream) {
ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, session->c,
@@ -519,9 +508,16 @@ static int on_frame_recv_cb(nghttp2_sess
}
break;
case NGHTTP2_GOAWAY:
- session->remote.accepted_max = frame->goaway.last_stream_id;
- session->remote.error = frame->goaway.error_code;
- dispatch_event(session, H2_SESSION_EV_REMOTE_GOAWAY, 0, NULL);
+ if (frame->goaway.error_code == 0
+ && frame->goaway.last_stream_id == ((1u << 31) - 1)) {
+ /* shutdown notice. Should not come from a client... */
+ session->remote.accepting = 0;
+ }
+ else {
+ session->remote.accepted_max = frame->goaway.last_stream_id;
+ dispatch_event(session, H2_SESSION_EV_REMOTE_GOAWAY,
+ frame->goaway.error_code, NULL);
+ }
break;
default:
if (APLOGctrace2(session->c)) {
@@ -715,12 +711,35 @@ static void h2_session_destroy(h2_sessio
}
}
+static apr_status_t h2_session_shutdown_notice(h2_session *session)
+{
+ apr_status_t status;
+
+ AP_DEBUG_ASSERT(session);
+ if (!session->local.accepting) {
+ return APR_SUCCESS;
+ }
+
+ nghttp2_submit_shutdown_notice(session->ngh2);
+ session->local.accepting = 0;
+ status = nghttp2_session_send(session->ngh2);
+ if (status == APR_SUCCESS) {
+ status = h2_conn_io_flush(&session->io);
+ }
+ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, APLOGNO()
+ "session(%ld): sent shutdown notice", session->id);
+ return status;
+}
+
static apr_status_t h2_session_shutdown(h2_session *session, int error,
const char *msg, int force_close)
{
apr_status_t status = APR_SUCCESS;
AP_DEBUG_ASSERT(session);
+ if (session->local.shutdown) {
+ return APR_SUCCESS;
+ }
if (!msg && error) {
msg = nghttp2_strerror(error);
}
@@ -744,6 +763,8 @@ static apr_status_t h2_session_shutdown(
nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE,
session->local.accepted_max,
error, (uint8_t*)msg, msg? strlen(msg):0);
+ session->local.accepting = 0;
+ session->local.shutdown = 1;
status = nghttp2_session_send(session->ngh2);
if (status == APR_SUCCESS) {
status = h2_conn_io_flush(&session->io);
@@ -773,8 +794,7 @@ static apr_status_t session_pool_cleanup
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
"session(%ld): pool_cleanup", session->id);
- if (session->state != H2_SESSION_ST_DONE
- && session->state != H2_SESSION_ST_LOCAL_SHUTDOWN) {
+ if (session->state != H2_SESSION_ST_DONE) {
/* Not good. The connection is being torn down and we have
* not sent a goaway. This is considered a protocol error and
* the client has to assume that any streams "in flight" may have
@@ -1596,9 +1616,6 @@ static apr_status_t h2_session_read(h2_s
* status. */
return rstatus;
}
- if (!is_accepting_streams(session)) {
- break;
- }
if ((session->io.bytes_read - read_start) > (64*1024)) {
/* read enough in one go, give write a chance */
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, c,
@@ -1649,8 +1666,6 @@ static const char *StateNames[] = {
"IDLE", /* H2_SESSION_ST_IDLE */
"BUSY", /* H2_SESSION_ST_BUSY */
"WAIT", /* H2_SESSION_ST_WAIT */
- "LSHUTDOWN", /* H2_SESSION_ST_LOCAL_SHUTDOWN */
- "RSHUTDOWN", /* H2_SESSION_ST_REMOTE_SHUTDOWN */
};
static const char *state_name(h2_session_state state)
@@ -1661,18 +1676,6 @@ static const char *state_name(h2_session
return StateNames[state];
}
-static int is_accepting_streams(h2_session *session)
-{
- switch (session->state) {
- case H2_SESSION_ST_IDLE:
- case H2_SESSION_ST_BUSY:
- case H2_SESSION_ST_WAIT:
- return 1;
- default:
- return 0;
- }
-}
-
static void update_child_status(h2_session *session, int status, const char *msg)
{
/* Assume that we also change code/msg when something really happened and
@@ -1709,12 +1712,6 @@ static void transit(h2_session *session,
SERVER_BUSY_KEEPALIVE
: SERVER_BUSY_READ), "idle");
break;
- case H2_SESSION_ST_REMOTE_SHUTDOWN:
- update_child_status(session, SERVER_CLOSING, "remote goaway");
- break;
- case H2_SESSION_ST_LOCAL_SHUTDOWN:
- update_child_status(session, SERVER_CLOSING, "local goaway");
- break;
case H2_SESSION_ST_DONE:
update_child_status(session, SERVER_CLOSING, "done");
break;
@@ -1739,39 +1736,22 @@ static void h2_session_ev_init(h2_sessio
static void h2_session_ev_local_goaway(h2_session *session, int arg, const char *msg)
{
- session->local.accepting = 0;
cleanup_streams(session);
- switch (session->state) {
- case H2_SESSION_ST_LOCAL_SHUTDOWN:
- /* already did that? */
- break;
- case H2_SESSION_ST_IDLE:
- case H2_SESSION_ST_REMOTE_SHUTDOWN:
- /* all done */
- transit(session, "local goaway", H2_SESSION_ST_DONE);
- break;
- default:
- transit(session, "local goaway", H2_SESSION_ST_LOCAL_SHUTDOWN);
- break;
+ if (!session->remote.shutdown) {
+ update_child_status(session, SERVER_CLOSING, "local goaway");
}
+ transit(session, "local goaway", H2_SESSION_ST_DONE);
}
static void h2_session_ev_remote_goaway(h2_session *session, int arg, const char *msg)
{
- session->remote.accepting = 0;
- cleanup_streams(session);
- switch (session->state) {
- case H2_SESSION_ST_REMOTE_SHUTDOWN:
- /* already received that? */
- break;
- case H2_SESSION_ST_IDLE:
- case H2_SESSION_ST_LOCAL_SHUTDOWN:
- /* all done */
- transit(session, "remote goaway", H2_SESSION_ST_DONE);
- break;
- default:
- transit(session, "remote goaway", H2_SESSION_ST_REMOTE_SHUTDOWN);
- break;
+ if (!session->remote.shutdown) {
+ session->remote.error = arg;
+ session->remote.accepting = 0;
+ session->remote.shutdown = 1;
+ cleanup_streams(session);
+ update_child_status(session, SERVER_CLOSING, "remote goaway");
+ transit(session, "remote goaway", H2_SESSION_ST_DONE);
}
}
@@ -1780,7 +1760,6 @@ static void h2_session_ev_conn_error(h2_
switch (session->state) {
case H2_SESSION_ST_INIT:
case H2_SESSION_ST_DONE:
- case H2_SESSION_ST_LOCAL_SHUTDOWN:
/* just leave */
transit(session, "conn error", H2_SESSION_ST_DONE);
break;
@@ -1795,31 +1774,18 @@ static void h2_session_ev_conn_error(h2_
static void h2_session_ev_proto_error(h2_session *session, int arg, const char *msg)
{
- switch (session->state) {
- case H2_SESSION_ST_DONE:
- case H2_SESSION_ST_LOCAL_SHUTDOWN:
- /* just leave */
- transit(session, "proto error", H2_SESSION_ST_DONE);
- break;
-
- default:
- ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, APLOGNO(03402)
- "h2_session(%ld): proto error -> shutdown", session->id);
- h2_session_shutdown(session, arg, msg, 0);
- break;
+ if (!session->local.shutdown) {
+ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, APLOGNO(03402)
+ "h2_session(%ld): proto error -> shutdown", session->id);
+ h2_session_shutdown(session, arg, msg, 0);
}
}
static void h2_session_ev_conn_timeout(h2_session *session, int arg, const char *msg)
{
- switch (session->state) {
- case H2_SESSION_ST_LOCAL_SHUTDOWN:
- transit(session, "conn timeout", H2_SESSION_ST_DONE);
- break;
- default:
- h2_session_shutdown(session, arg, msg, 1);
- transit(session, "conn timeout", H2_SESSION_ST_DONE);
- break;
+ transit(session, msg, H2_SESSION_ST_DONE);
+ if (!session->local.shutdown) {
+ h2_session_shutdown(session, arg, msg, 1);
}
}
@@ -1827,8 +1793,6 @@ static void h2_session_ev_no_io(h2_sessi
{
switch (session->state) {
case H2_SESSION_ST_BUSY:
- case H2_SESSION_ST_LOCAL_SHUTDOWN:
- case H2_SESSION_ST_REMOTE_SHUTDOWN:
/* Nothing to READ, nothing to WRITE on the master connection.
* Possible causes:
* - we wait for the client to send us sth
@@ -1838,6 +1802,7 @@ static void h2_session_ev_no_io(h2_sessi
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, session->c,
"h2_session(%ld): NO_IO event, %d streams open",
session->id, session->open_streams);
+ h2_conn_io_flush(&session->io);
if (session->open_streams > 0) {
if (has_unsubmitted_streams(session)
|| has_suspended_streams(session)) {
@@ -1861,7 +1826,7 @@ static void h2_session_ev_no_io(h2_sessi
}
}
}
- else if (is_accepting_streams(session)) {
+ else if (session->local.accepting) {
/* When we have no streams, but accept new, switch to idle */
apr_time_t now = apr_time_now();
transit(session, "no io (keepalive)", H2_SESSION_ST_IDLE);
@@ -1924,26 +1889,17 @@ static void h2_session_ev_mpm_stopping(h
{
switch (session->state) {
case H2_SESSION_ST_DONE:
- case H2_SESSION_ST_LOCAL_SHUTDOWN:
/* nop */
break;
default:
- h2_session_shutdown(session, arg, msg, 0);
+ h2_session_shutdown_notice(session);
break;
}
}
static void h2_session_ev_pre_close(h2_session *session, int arg, const char *msg)
{
- switch (session->state) {
- case H2_SESSION_ST_DONE:
- case H2_SESSION_ST_LOCAL_SHUTDOWN:
- /* nop */
- break;
- default:
- h2_session_shutdown(session, arg, msg, 1);
- break;
- }
+ h2_session_shutdown(session, arg, msg, 1);
}
static void h2_session_ev_stream_open(h2_session *session, int arg, const char *msg)
@@ -2053,14 +2009,14 @@ apr_status_t h2_session_process(h2_sessi
c->cs->state = CONN_STATE_WRITE_COMPLETION;
}
- while (1) {
+ while (session->state != H2_SESSION_ST_DONE) {
trace = APLOGctrace3(c);
session->have_read = session->have_written = 0;
- if (!ap_mpm_query(AP_MPMQ_MPM_STATE, &mpm_state)) {
+ if (session->local.accepting
+ && !ap_mpm_query(AP_MPMQ_MPM_STATE, &mpm_state)) {
if (mpm_state == AP_MPMQ_STOPPING) {
dispatch_event(session, H2_SESSION_EV_MPM_STOPPING, 0, NULL);
- break;
}
}
@@ -2189,8 +2145,6 @@ apr_status_t h2_session_process(h2_sessi
break;
case H2_SESSION_ST_BUSY:
- case H2_SESSION_ST_LOCAL_SHUTDOWN:
- case H2_SESSION_ST_REMOTE_SHUTDOWN:
if (nghttp2_session_want_read(session->ngh2)) {
ap_update_child_status(session->c->sbh, SERVER_BUSY_READ, NULL);
h2_filter_cin_timeout_set(session->cin, session->s->timeout);
@@ -2273,7 +2227,7 @@ apr_status_t h2_session_process(h2_sessi
else if (APR_STATUS_IS_TIMEUP(status)) {
/* go back to checking all inputs again */
transit(session, "wait cycle", session->local.accepting?
- H2_SESSION_ST_BUSY : H2_SESSION_ST_LOCAL_SHUTDOWN);
+ H2_SESSION_ST_BUSY : H2_SESSION_ST_DONE);
}
else if (APR_STATUS_IS_ECONNRESET(status)
|| APR_STATUS_IS_ECONNABORTED(status)) {
@@ -2289,10 +2243,6 @@ apr_status_t h2_session_process(h2_sessi
}
break;
- case H2_SESSION_ST_DONE:
- status = APR_EOF;
- goto out;
-
default:
ap_log_cerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, c,
APLOGNO(03080)
@@ -2322,11 +2272,12 @@ out:
&& (APR_STATUS_IS_EOF(status)
|| APR_STATUS_IS_ECONNRESET(status)
|| APR_STATUS_IS_ECONNABORTED(status))) {
- dispatch_event(session, H2_SESSION_EV_CONN_ERROR, 0, NULL);
- }
+ dispatch_event(session, H2_SESSION_EV_CONN_ERROR, 0, NULL);
+ }
- status = (session->state == H2_SESSION_ST_DONE)? APR_EOF : APR_SUCCESS;
+ status = APR_SUCCESS;
if (session->state == H2_SESSION_ST_DONE) {
+ status = APR_EOF;
if (!session->eoc_written) {
session->eoc_written = 1;
h2_conn_io_write_eoc(&session->io, session);
@@ -2340,6 +2291,6 @@ apr_status_t h2_session_pre_close(h2_ses
{
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
"h2_session(%ld): pre_close", session->id);
- dispatch_event(session, H2_SESSION_EV_PRE_CLOSE, 0, "timeout");
+ dispatch_event(session, H2_SESSION_EV_PRE_CLOSE, 0, NULL);
return APR_SUCCESS;
}