Robust GOAWAY handling

This change will utilize last_stream_id in GOAWAY extensively.  When
GOAWAY is received with a last_stream_id, library closes all outgoing
streams whose stream_id > received last_stream_id.
nghttp2_on_stream_callback is called for each stream to be closed.

When GOAWAY is sent with a last_stream_id, library closes all incoming
streams whose stream_id > sent last_stream_id.
nghttp2_on_stream_callback is called for each stream to be closed.

lib/includes/nghttp2/nghttp2.h

@@ -2251,8 +2251,10 @@ int nghttp2_session_get_stream_remote_close(nghttp2_session *session,
  *
  * Signals the session so that the connection should be terminated.
  *
- * The last stream ID is the ID of a stream for which
- * :type:`nghttp2_on_frame_recv_callback` was called most recently.
+ * The last stream ID is the minimum value between the stream ID of a
+ * stream for which :type:`nghttp2_on_frame_recv_callback` was called
+ * most recently and the last stream ID we have sent to the peer
+ * previously.
  *
  * The |error_code| is the error code of this GOAWAY frame.  The
  * pre-defined error code is one of :enum:`nghttp2_error_code`.
@@ -2280,13 +2282,24 @@ int nghttp2_session_terminate_session(nghttp2_session *session,
  *
  * This function behaves like `nghttp2_session_terminate_session()`,
  * but the last stream ID can be specified by the application for fine
- * grained control of stream.
+ * grained control of stream.  The HTTP/2 specification does not allow
+ * last_stream_id to be increased.  So the actual value sent as
+ * last_stream_id is the minimum value between the given
+ * |last_stream_id| and the last_stream_id we have previously sent to
+ * the peer.
+ *
+ * The |last_stream_id| is peer's stream ID or 0.  So if |session| is
+ * initialized as client, |last_stream_id| must be even or 0.  If
+ * |session| is initialized as server, |last_stream_id| must be odd or
+ * 0.
  *
  * This function returns 0 if it succeeds, or one of the following
  * negative error codes:
  *
  * :enum:`NGHTTP2_ERR_NOMEM`
  *     Out of memory.
+ * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT`
+ *     The |last_stream_id| is invalid.
  */
 int nghttp2_session_terminate_session2(nghttp2_session *session,
                                        int32_t last_stream_id,
@@ -2840,6 +2853,17 @@ int nghttp2_submit_ping(nghttp2_session *session, uint8_t flags,
  * The |flags| is currently ignored and should be
  * :enum:`NGHTTP2_FLAG_NONE`.
  *
+ * The |last_stream_id| is peer's stream ID or 0.  So if |session| is
+ * initialized as client, |last_stream_id| must be even or 0.  If
+ * |session| is initialized as server, |last_stream_id| must be odd or
+ * 0.
+ *
+ * The HTTP/2 specification says last_stream_id must not be increased
+ * from the value previously sent.  So the actual value sent as
+ * last_stream_id is the minimum value between the given
+ * |last_stream_id| and the last_stream_id previously sent to the
+ * peer.
+ *
  * If the |opaque_data| is not ``NULL`` and |opaque_data_len| is not
  * zero, those data will be sent as additional debug data.  The
  * library makes a copy of the memory region pointed by |opaque_data|
@@ -2847,19 +2871,14 @@ int nghttp2_submit_ping(nghttp2_session *session, uint8_t flags,
  * keep this memory after the return of this function.  If the
  * |opaque_data_len| is 0, the |opaque_data| could be ``NULL``.
  *
- * To shutdown gracefully, first send GOAWAY with ``last_stream_id =
- * (1u << 31) - 1``.  After 1 RTT, call either
- * `nghttp2_submit_goaway()`, `nghttp2_session_terminate_session()` or
- * `nghttp2_session_terminate_session2()`.  The latter 2 will close
- * HTTP/2 session immediately after transmission of the frame.
- *
  * This function returns 0 if it succeeds, or one of the following
  * negative error codes:
  *
  * :enum:`NGHTTP2_ERR_NOMEM`
  *     Out of memory.
  * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT`
- *     The |opaque_data_len| is too large.
+ *     The |opaque_data_len| is too large; the |last_stream_id| is
+ *     invalid.
  */
 int nghttp2_submit_goaway(nghttp2_session *session, uint8_t flags,
                           int32_t last_stream_id, uint32_t error_code,

lib/nghttp2_outbound_item.h

@@ -69,10 +69,18 @@ typedef struct {
   uint8_t eof;
 } nghttp2_data_aux_data;
 
+/* struct used for GOAWAY frame */
+typedef struct {
+  /* nonzero if session should be terminated after the transmission of
+     this frame. */
+  int terminate_on_send;
+} nghttp2_goaway_aux_data;
+
 /* Additional data which cannot be stored in nghttp2_frame struct */
 typedef union {
   nghttp2_data_aux_data data;
   nghttp2_headers_aux_data headers;
+  nghttp2_goaway_aux_data goaway;
 } nghttp2_aux_data;
 
 typedef struct {

lib/nghttp2_session.h

@@ -126,12 +126,10 @@ typedef struct {
 
 typedef enum {
   NGHTTP2_GOAWAY_NONE = 0,
-  /* Flag means GOAWAY frame is sent to the remote peer. */
-  NGHTTP2_GOAWAY_SEND = 0x1,
-  /* Flag means GOAWAY frame is received from the remote peer. */
-  NGHTTP2_GOAWAY_RECV = 0x2,
-  /* Flag means connection should be dropped after sending GOAWAY. */
-  NGHTTP2_GOAWAY_FAIL_ON_SEND = 0x4
+  /* Flag means that connection should be terminated after sending GOAWAY. */
+  NGHTTP2_GOAWAY_FAIL_ON_SEND = 0x1,
+  /* Flag means GOAWAY to terminate session has been sent */
+  NGHTTP2_GOAWAY_TERM_SENT = 0x2,
 } nghttp2_goaway_flag;
 
 struct nghttp2_session {
@@ -250,6 +248,17 @@ typedef struct {
   int32_t new_window_size, old_window_size;
 } nghttp2_update_window_size_arg;
 
+typedef struct {
+  nghttp2_session *session;
+  /* linked list of streams to close */
+  nghttp2_stream *head;
+  int32_t last_stream_id;
+  /* nonzero if GOAWAY is sent to peer, which means we are going to
+     close incoming streams.  zero if GOAWAY is received from peer and
+     we are going to close outgoing streams. */
+  int incoming;
+} nghttp2_close_stream_on_goaway_arg;
+
 /* TODO stream timeout etc */
 
 /*
@@ -333,7 +342,7 @@ int nghttp2_session_add_ping(nghttp2_session *session, uint8_t flags,
  */
 int nghttp2_session_add_goaway(nghttp2_session *session, int32_t last_stream_id,
                                uint32_t error_code, const uint8_t *opaque_data,
-                               size_t opaque_data_len);
+                               size_t opaque_data_len, int terminate_on_send);
 
 /*
  * Adds WINDOW_UPDATE frame with stream ID |stream_id| and

lib/nghttp2_submit.c

@@ -228,7 +228,7 @@ int nghttp2_submit_goaway(nghttp2_session *session, uint8_t flags _U_,
                           int32_t last_stream_id, uint32_t error_code,
                           const uint8_t *opaque_data, size_t opaque_data_len) {
   return nghttp2_session_add_goaway(session, last_stream_id, error_code,
-                                    opaque_data, opaque_data_len);
+                                    opaque_data, opaque_data_len, 0);
 }
 
 int nghttp2_submit_settings(nghttp2_session *session, uint8_t flags _U_,

tests/nghttp2_session_test.c

@@ -345,13 +345,13 @@ static ssize_t defer_data_source_read_callback(nghttp2_session *session _U_,
   return NGHTTP2_ERR_DEFERRED;
 }
 
-static int stream_close_callback(nghttp2_session *session, int32_t stream_id,
+static int on_stream_close_callback(nghttp2_session *session _U_,
+                                    int32_t stream_id _U_,
                                     nghttp2_error_code error_code _U_,
                                     void *user_data) {
   my_user_data *my_data = (my_user_data *)user_data;
-  void *stream_data = nghttp2_session_get_stream_user_data(session, stream_id);
   ++my_data->stream_close_cb_called;
-  CU_ASSERT(stream_data != NULL);
+
   return 0;
 }
 
@@ -1819,6 +1819,33 @@ void test_nghttp2_session_on_request_headers_received(void) {
   nghttp2_frame_headers_free(&frame.headers);
 
   nghttp2_session_del(session);
+
+  nghttp2_session_server_new(&session, &callbacks, &user_data);
+
+  /* Stream ID which is equal to local_last_stream_id is ok. */
+  session->local_last_stream_id = 3;
+
+  nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 3,
+                             NGHTTP2_HCAT_REQUEST, NULL, NULL, 0);
+
+  CU_ASSERT(0 == nghttp2_session_on_request_headers_received(session, &frame));
+
+  nghttp2_frame_headers_free(&frame.headers);
+
+  /* Stream ID which is greater than local_last_stream_id is
+     ignored */
+  nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 5,
+                             NGHTTP2_HCAT_REQUEST, NULL, NULL, 0);
+
+  user_data.invalid_frame_recv_cb_called = 0;
+  CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK ==
+            nghttp2_session_on_request_headers_received(session, &frame));
+  CU_ASSERT(0 == user_data.invalid_frame_recv_cb_called);
+  CU_ASSERT(0 == (session->goaway_flags & NGHTTP2_GOAWAY_FAIL_ON_SEND));
+
+  nghttp2_frame_headers_free(&frame.headers);
+
+  nghttp2_session_del(session);
 }
 
 void test_nghttp2_session_on_response_headers_received(void) {
@@ -2471,19 +2498,40 @@ void test_nghttp2_session_on_goaway_received(void) {
   nghttp2_session_callbacks callbacks;
   my_user_data user_data;
   nghttp2_frame frame;
+  int i;
+
   user_data.frame_recv_cb_called = 0;
   user_data.invalid_frame_recv_cb_called = 0;
 
   memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
   callbacks.on_frame_recv_callback = on_frame_recv_callback;
   callbacks.on_invalid_frame_recv_callback = on_invalid_frame_recv_callback;
+  callbacks.on_stream_close_callback = on_stream_close_callback;
 
   nghttp2_session_client_new(&session, &callbacks, &user_data);
-  nghttp2_frame_goaway_init(&frame.goaway, 1, NGHTTP2_PROTOCOL_ERROR, NULL, 0);
+
+  for (i = 1; i <= 7; ++i) {
+    open_stream(session, i);
+  }
+
+  nghttp2_frame_goaway_init(&frame.goaway, 3, NGHTTP2_PROTOCOL_ERROR, NULL, 0);
+
+  user_data.stream_close_cb_called = 0;
 
   CU_ASSERT(0 == nghttp2_session_on_goaway_received(session, &frame));
+
   CU_ASSERT(1 == user_data.frame_recv_cb_called);
-  CU_ASSERT(session->goaway_flags == NGHTTP2_GOAWAY_RECV);
+  CU_ASSERT(3 == session->remote_last_stream_id);
+  /* on_stream_close should be callsed for 2 times (stream 5 and 7) */
+  CU_ASSERT(2 == user_data.stream_close_cb_called);
+
+  CU_ASSERT(NULL != nghttp2_session_get_stream(session, 1));
+  CU_ASSERT(NULL != nghttp2_session_get_stream(session, 2));
+  CU_ASSERT(NULL != nghttp2_session_get_stream(session, 3));
+  CU_ASSERT(NULL != nghttp2_session_get_stream(session, 4));
+  CU_ASSERT(NULL == nghttp2_session_get_stream(session, 5));
+  CU_ASSERT(NULL != nghttp2_session_get_stream(session, 6));
+  CU_ASSERT(NULL == nghttp2_session_get_stream(session, 7));
 
   nghttp2_frame_goaway_free(&frame.goaway);
   nghttp2_session_del(session);
@@ -4855,7 +4903,7 @@ void test_nghttp2_session_on_stream_close(void) {
   nghttp2_stream *stream;
 
   memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
-  callbacks.on_stream_close_callback = stream_close_callback;
+  callbacks.on_stream_close_callback = on_stream_close_callback;
   user_data.stream_close_cb_called = 0;
 
   nghttp2_session_client_new(&session, &callbacks, &user_data);
@@ -4917,10 +4965,6 @@ void test_nghttp2_session_on_ctrl_not_send(void) {
   CU_ASSERT(NGHTTP2_HEADERS == user_data.not_sent_frame_type);
   CU_ASSERT(NGHTTP2_ERR_STREAM_CLOSING == user_data.not_sent_error);
 
-  stream = nghttp2_session_open_stream(session, 3, NGHTTP2_STREAM_FLAG_NONE,
-                                       &pri_spec_default, NGHTTP2_STREAM_OPENED,
-                                       &user_data);
-
   nghttp2_session_del(session);
 
   /* Check request HEADERS */
@@ -4933,10 +4977,9 @@ void test_nghttp2_session_on_ctrl_not_send(void) {
                                    NULL, 0, NULL));
 
   user_data.frame_not_send_cb_called = 0;
-  /* Send GOAWAY */
-  CU_ASSERT(0 == nghttp2_submit_goaway(session, NGHTTP2_FLAG_NONE,
-                                       (1u << 31) - 1, NGHTTP2_NO_ERROR, NULL,
-                                       0));
+  /* Terminating session */
+  CU_ASSERT(0 == nghttp2_session_add_goaway(session, 0, NGHTTP2_NO_ERROR, NULL,
+                                            0, 1));
 
   session->next_stream_id = 9;
 
@@ -4961,7 +5004,7 @@ void test_nghttp2_session_get_outbound_queue_size(void) {
   CU_ASSERT(0 == nghttp2_submit_ping(session, NGHTTP2_FLAG_NONE, NULL));
   CU_ASSERT(1 == nghttp2_session_get_outbound_queue_size(session));
 
-  CU_ASSERT(0 == nghttp2_submit_goaway(session, NGHTTP2_FLAG_NONE, 3,
+  CU_ASSERT(0 == nghttp2_submit_goaway(session, NGHTTP2_FLAG_NONE, 2,
                                        NGHTTP2_NO_ERROR, NULL, 0));
   CU_ASSERT(2 == nghttp2_session_get_outbound_queue_size(session));
 
@@ -6401,9 +6444,15 @@ void test_nghttp2_session_graceful_shutdown(void) {
   memset(&callbacks, 0, sizeof(callbacks));
   callbacks.send_callback = block_count_send_callback;
   callbacks.on_frame_send_callback = on_frame_send_callback;
+  callbacks.on_stream_close_callback = on_stream_close_callback;
 
   nghttp2_session_server_new(&session, &callbacks, &ud);
 
+  open_stream(session, 301);
+  open_stream(session, 302);
+  open_stream(session, 311);
+  open_stream(session, 319);
+
   CU_ASSERT(0 == nghttp2_submit_goaway(session, NGHTTP2_FLAG_NONE,
                                        (1u << 31) - 1, NGHTTP2_NO_ERROR, NULL,
                                        0));
@@ -6417,15 +6466,22 @@ void test_nghttp2_session_graceful_shutdown(void) {
   CU_ASSERT((1u << 31) - 1 == session->local_last_stream_id);
 
   CU_ASSERT(0 ==
-            nghttp2_session_terminate_session2(session, 300, NGHTTP2_NO_ERROR));
+            nghttp2_session_terminate_session2(session, 301, NGHTTP2_NO_ERROR));
 
   ud.block_count = 1;
   ud.frame_send_cb_called = 0;
+  ud.stream_close_cb_called = 0;
 
   CU_ASSERT(0 == nghttp2_session_send(session));
 
   CU_ASSERT(1 == ud.frame_send_cb_called);
-  CU_ASSERT(300 == session->local_last_stream_id);
+  CU_ASSERT(301 == session->local_last_stream_id);
+  CU_ASSERT(2 == ud.stream_close_cb_called);
+
+  CU_ASSERT(NULL != nghttp2_session_get_stream(session, 301));
+  CU_ASSERT(NULL != nghttp2_session_get_stream(session, 302));
+  CU_ASSERT(NULL == nghttp2_session_get_stream(session, 311));
+  CU_ASSERT(NULL == nghttp2_session_get_stream(session, 319));
 
   nghttp2_session_del(session);
 }