From b0078a2379e28c4e58774690329d23e433eecb1e Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 15 Jan 2015 22:32:29 +0900 Subject: [PATCH] Suppress to send frames other than GOAWAY if NGHTTP2_GOAWAY_TERM_ON_SEND is set This change makes sure that GOAWAY which terminates session is immediately sent without blocked by other frames. NGHTTP2_ERR_SESSION_CLOSING library error code was added to indicate this situation to callback. --- lib/includes/nghttp2/nghttp2.h | 5 ++ lib/nghttp2_helper.c | 2 + lib/nghttp2_session.c | 90 ++++++++++++++++++++-------------- 3 files changed, 59 insertions(+), 38 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 46933e2c..ef4ed194 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -325,6 +325,11 @@ typedef enum { * not been fully processed yet. */ NGHTTP2_ERR_DATA_EXIST = -529, + /** + * The current session is closing due to a connection error or + * `nghttp2_session_terminate_session()` is called. + */ + NGHTTP2_ERR_SESSION_CLOSING = -530, /** * The errors < :enum:`NGHTTP2_ERR_FATAL` mean that the library is * under unexpected condition and processing was terminated (e.g., diff --git a/lib/nghttp2_helper.c b/lib/nghttp2_helper.c index 1be8d0dc..4b59e634 100644 --- a/lib/nghttp2_helper.c +++ b/lib/nghttp2_helper.c @@ -267,6 +267,8 @@ const char *nghttp2_strerror(int error_code) { return "Invalid stream state"; case NGHTTP2_ERR_DEFERRED_DATA_EXIST: return "Another DATA frame has already been deferred"; + case NGHTTP2_ERR_SESSION_CLOSING: + return "The current session is closing"; case NGHTTP2_ERR_START_STREAM_NOT_ALLOWED: return "request HEADERS is not allowed"; case NGHTTP2_ERR_GOAWAY_ALREADY_SENT: diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 3da4b6a5..f07e78ac 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -1214,6 +1214,13 @@ int nghttp2_session_close_stream_if_shut_rdwr(nghttp2_session *session, return 0; } +/* + * This function returns nonzero if session is closing. + */ +static int session_is_closing(nghttp2_session *session) { + return (session->goaway_flags & NGHTTP2_GOAWAY_TERM_ON_SEND) != 0; +} + /* * Check that we can send a frame to the |stream|. This function * returns 0 if we can send a frame to the |frame|, or one of the @@ -1223,11 +1230,17 @@ int nghttp2_session_close_stream_if_shut_rdwr(nghttp2_session *session, * The stream is already closed. * NGHTTP2_ERR_STREAM_SHUT_WR * The stream is half-closed for transmission. + * NGHTTP2_ERR_SESSION_CLOSING + * This session is closing. */ -static int stream_predicate_for_send(nghttp2_stream *stream) { +static int session_predicate_for_stream_send(nghttp2_session *session, + nghttp2_stream *stream) { if (stream == NULL) { return NGHTTP2_ERR_STREAM_CLOSED; } + if (session_is_closing(session)) { + return NGHTTP2_ERR_SESSION_CLOSING; + } if (stream->shut_flags & NGHTTP2_SHUT_WR) { return NGHTTP2_ERR_STREAM_SHUT_WR; } @@ -1275,11 +1288,13 @@ static int session_predicate_request_headers_send(nghttp2_session *session) { * RST_STREAM was queued for this stream. * NGHTTP2_ERR_INVALID_STREAM_STATE * The state of the stream is not valid. + * NGHTTP2_ERR_SESSION_CLOSING + * This session is closing. */ static int session_predicate_response_headers_send(nghttp2_session *session, nghttp2_stream *stream) { int rv; - rv = stream_predicate_for_send(stream); + rv = session_predicate_for_stream_send(session, stream); if (rv != 0) { return rv; } @@ -1312,13 +1327,15 @@ static int session_predicate_response_headers_send(nghttp2_session *session, * The stream is not reserved state * NGHTTP2_ERR_STREAM_CLOSED * RST_STREAM was queued for this stream. + * NGHTTP2_ERR_SESSION_CLOSING + * This session is closing. */ static int -session_predicate_push_response_headers_send(nghttp2_session *session _U_, +session_predicate_push_response_headers_send(nghttp2_session *session, nghttp2_stream *stream) { int rv; /* TODO Should disallow HEADERS if GOAWAY has already been issued? */ - rv = stream_predicate_for_send(stream); + rv = session_predicate_for_stream_send(session, stream); if (rv != 0) { return rv; } @@ -1333,7 +1350,8 @@ session_predicate_push_response_headers_send(nghttp2_session *session _U_, } /* - * This function checks frames belongs to the |stream| can be sent. + * This function checks HEADERS, which is neither stream-opening nor + * first response header, with the |stream| can be sent at this time. * The |stream| can be NULL. * * This function returns 0 if it succeeds, or one of the following @@ -1348,11 +1366,13 @@ session_predicate_push_response_headers_send(nghttp2_session *session _U_, * RST_STREAM was queued for this stream. * NGHTTP2_ERR_INVALID_STREAM_STATE * The state of the stream is not valid. + * NGHTTP2_ERR_SESSION_CLOSING + * This session is closing. */ -static int session_predicate_stream_frame_send(nghttp2_session *session, - nghttp2_stream *stream) { +static int session_predicate_headers_send(nghttp2_session *session, + nghttp2_stream *stream) { int rv; - rv = stream_predicate_for_send(stream); + rv = session_predicate_for_stream_send(session, stream); if (rv != 0) { return rv; } @@ -1372,16 +1392,6 @@ static int session_predicate_stream_frame_send(nghttp2_session *session, return NGHTTP2_ERR_INVALID_STREAM_STATE; } -/* - * This function checks HEADERS, which is neither stream-opening nor - * first response header, with the |stream| can be sent at this time. - * The |stream| can be NULL. - */ -static int session_predicate_headers_send(nghttp2_session *session, - nghttp2_stream *stream) { - return session_predicate_stream_frame_send(session, stream); -} - /* * This function checks PUSH_PROMISE frame |frame| with the |stream| * can be sent at this time. The |stream| can be NULL. @@ -1404,6 +1414,8 @@ static int session_predicate_headers_send(nghttp2_session *session, * with END_STREAM flag set has already sent) * NGHTTP2_ERR_PUSH_DISABLED * The remote peer disabled reception of PUSH_PROMISE. + * NGHTTP2_ERR_SESSION_CLOSING + * This session is closing. */ static int session_predicate_push_promise_send(nghttp2_session *session, nghttp2_stream *stream) { @@ -1413,7 +1425,7 @@ static int session_predicate_push_promise_send(nghttp2_session *session, return NGHTTP2_ERR_PROTO; } - rv = stream_predicate_for_send(stream); + rv = session_predicate_for_stream_send(session, stream); if (rv != 0) { return rv; } @@ -1426,8 +1438,7 @@ static int session_predicate_push_promise_send(nghttp2_session *session, if (stream->state == NGHTTP2_STREAM_CLOSING) { return NGHTTP2_ERR_STREAM_CLOSING; } - if (session->goaway_flags & - (NGHTTP2_GOAWAY_TERM_ON_SEND | NGHTTP2_GOAWAY_RECV)) { + if (session->goaway_flags & NGHTTP2_GOAWAY_RECV) { return NGHTTP2_ERR_START_STREAM_NOT_ALLOWED; } return 0; @@ -1447,6 +1458,8 @@ static int session_predicate_push_promise_send(nghttp2_session *session, * RST_STREAM was queued for this stream. * NGHTTP2_ERR_INVALID_STREAM_STATE * The state of the stream is not valid. + * NGHTTP2_ERR_SESSION_CLOSING + * This session is closing. */ static int session_predicate_window_update_send(nghttp2_session *session, int32_t stream_id) { @@ -1459,6 +1472,9 @@ static int session_predicate_window_update_send(nghttp2_session *session, if (stream == NULL) { return NGHTTP2_ERR_STREAM_CLOSED; } + if (session_is_closing(session)) { + return NGHTTP2_ERR_SESSION_CLOSING; + } if (stream->state == NGHTTP2_STREAM_CLOSING) { return NGHTTP2_ERR_STREAM_CLOSING; } @@ -1468,16 +1484,6 @@ static int session_predicate_window_update_send(nghttp2_session *session, return 0; } -/* - * This function checks SETTINGS can be sent at this time. - * - * Currently this function always returns 0. - */ -static int nghttp2_session_predicate_settings_send(nghttp2_session *session _U_, - nghttp2_frame *frame _U_) { - return 0; -} - /* Take into account settings max frame size and both connection-level flow control here */ static ssize_t @@ -1530,11 +1536,13 @@ static size_t nghttp2_session_next_data_read(nghttp2_session *session, * RST_STREAM was queued for this stream. * NGHTTP2_ERR_INVALID_STREAM_STATE * The state of the stream is not valid. + * NGHTTP2_ERR_SESSION_CLOSING + * This session is closing. */ static int nghttp2_session_predicate_data_send(nghttp2_session *session, nghttp2_stream *stream) { int rv; - rv = stream_predicate_for_send(stream); + rv = session_predicate_for_stream_send(session, stream); if (rv != 0) { return rv; } @@ -1759,6 +1767,9 @@ static int session_prep_frame(nghttp2_session *session, return framerv; } case NGHTTP2_PRIORITY: { + if (session_is_closing(session)) { + return NGHTTP2_ERR_SESSION_CLOSING; + } /* PRIORITY frame can be sent at any time and to any stream ID. */ framerv = nghttp2_frame_pack_priority(&session->aob.framebufs, @@ -1774,6 +1785,9 @@ static int session_prep_frame(nghttp2_session *session, break; } case NGHTTP2_RST_STREAM: + if (session_is_closing(session)) { + return NGHTTP2_ERR_SESSION_CLOSING; + } framerv = nghttp2_frame_pack_rst_stream(&session->aob.framebufs, &frame->rst_stream); if (framerv < 0) { @@ -1781,10 +1795,6 @@ static int session_prep_frame(nghttp2_session *session, } break; case NGHTTP2_SETTINGS: { - rv = nghttp2_session_predicate_settings_send(session, frame); - if (rv != 0) { - return rv; - } framerv = nghttp2_frame_pack_settings(&session->aob.framebufs, &frame->settings); if (framerv < 0) { @@ -1840,6 +1850,9 @@ static int session_prep_frame(nghttp2_session *session, break; } case NGHTTP2_PING: + if (session_is_closing(session)) { + return NGHTTP2_ERR_SESSION_CLOSING; + } framerv = nghttp2_frame_pack_ping(&session->aob.framebufs, &frame->ping); if (framerv < 0) { return framerv; @@ -3829,7 +3842,7 @@ int nghttp2_session_on_settings_received(nghttp2_session *session, } } - if (!noack) { + if (!noack && !session_is_closing(session)) { rv = nghttp2_session_add_settings(session, NGHTTP2_FLAG_ACK, NULL, 0); if (rv != 0) { @@ -3997,7 +4010,8 @@ int nghttp2_session_on_ping_received(nghttp2_session *session, return session_handle_invalid_connection( session, frame, NGHTTP2_PROTOCOL_ERROR, "PING: stream_id != 0"); } - if ((frame->hd.flags & NGHTTP2_FLAG_ACK) == 0) { + if ((frame->hd.flags & NGHTTP2_FLAG_ACK) == 0 && + !session_is_closing(session)) { /* Peer sent ping, so ping it back */ rv = nghttp2_session_add_ping(session, NGHTTP2_FLAG_ACK, frame->ping.opaque_data);