From 699f810cf234f97fad1d08e8cb57a7b9df10aa37 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 20 Jun 2019 19:51:12 -0700 Subject: [PATCH 1/2] Do not create streams after a GOAWAY has been received --- .../chttp2/transport/chttp2_transport.cc | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index f4b0f694142..7d6d6a80745 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -1198,10 +1198,30 @@ void grpc_chttp2_add_incoming_goaway(grpc_chttp2_transport* t, static void maybe_start_some_streams(grpc_chttp2_transport* t) { grpc_chttp2_stream* s; + /* cancel out streams that will never be started */ + if (t->goaway_error != GRPC_ERROR_NONE) { + while (grpc_chttp2_list_pop_waiting_for_concurrency(t, &s)) { + grpc_chttp2_cancel_stream( + t, s, + grpc_error_set_int( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("GOAWAY received"), + GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE)); + } + return; + } + if (t->next_stream_id >= MAX_CLIENT_STREAM_ID) { + while (grpc_chttp2_list_pop_waiting_for_concurrency(t, &s)) { + grpc_chttp2_cancel_stream( + t, s, + grpc_error_set_int( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("Stream IDs exhausted"), + GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE)); + } + return; + } /* start streams where we have free grpc_chttp2_stream ids and free * concurrency */ - while (t->next_stream_id <= MAX_CLIENT_STREAM_ID && - grpc_chttp2_stream_map_size(&t->stream_map) < + while (grpc_chttp2_stream_map_size(&t->stream_map) < t->settings[GRPC_PEER_SETTINGS] [GRPC_CHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS] && grpc_chttp2_list_pop_waiting_for_concurrency(t, &s)) { @@ -1225,15 +1245,6 @@ static void maybe_start_some_streams(grpc_chttp2_transport* t) { grpc_chttp2_mark_stream_writable(t, s); grpc_chttp2_initiate_write(t, GRPC_CHTTP2_INITIATE_WRITE_START_NEW_STREAM); } - /* cancel out streams that will never be started */ - while (t->next_stream_id >= MAX_CLIENT_STREAM_ID && - grpc_chttp2_list_pop_waiting_for_concurrency(t, &s)) { - grpc_chttp2_cancel_stream( - t, s, - grpc_error_set_int( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Stream IDs exhausted"), - GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE)); - } } /* Flag that this closure barrier may be covering a write in a pollset, and so From 4076695969c645e7b487656c9aa61e2359b7e3f7 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 15 Aug 2019 16:48:37 -0700 Subject: [PATCH 2/2] Reviewer comments --- .../chttp2/transport/chttp2_transport.cc | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 7d6d6a80745..73235756dfb 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -1198,7 +1198,7 @@ void grpc_chttp2_add_incoming_goaway(grpc_chttp2_transport* t, static void maybe_start_some_streams(grpc_chttp2_transport* t) { grpc_chttp2_stream* s; - /* cancel out streams that will never be started */ + /* cancel out streams that haven't yet started if we have received a GOAWAY */ if (t->goaway_error != GRPC_ERROR_NONE) { while (grpc_chttp2_list_pop_waiting_for_concurrency(t, &s)) { grpc_chttp2_cancel_stream( @@ -1209,19 +1209,10 @@ static void maybe_start_some_streams(grpc_chttp2_transport* t) { } return; } - if (t->next_stream_id >= MAX_CLIENT_STREAM_ID) { - while (grpc_chttp2_list_pop_waiting_for_concurrency(t, &s)) { - grpc_chttp2_cancel_stream( - t, s, - grpc_error_set_int( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Stream IDs exhausted"), - GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE)); - } - return; - } /* start streams where we have free grpc_chttp2_stream ids and free * concurrency */ - while (grpc_chttp2_stream_map_size(&t->stream_map) < + while (t->next_stream_id <= MAX_CLIENT_STREAM_ID && + grpc_chttp2_stream_map_size(&t->stream_map) < t->settings[GRPC_PEER_SETTINGS] [GRPC_CHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS] && grpc_chttp2_list_pop_waiting_for_concurrency(t, &s)) { @@ -1245,6 +1236,16 @@ static void maybe_start_some_streams(grpc_chttp2_transport* t) { grpc_chttp2_mark_stream_writable(t, s); grpc_chttp2_initiate_write(t, GRPC_CHTTP2_INITIATE_WRITE_START_NEW_STREAM); } + /* cancel out streams that will never be started */ + if (t->next_stream_id >= MAX_CLIENT_STREAM_ID) { + while (grpc_chttp2_list_pop_waiting_for_concurrency(t, &s)) { + grpc_chttp2_cancel_stream( + t, s, + grpc_error_set_int( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("Stream IDs exhausted"), + GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE)); + } + } } /* Flag that this closure barrier may be covering a write in a pollset, and so