From edf3d5e589f8811b04967f01c04f50d9168438ac Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 20 Jul 2020 19:10:08 -0700 Subject: [PATCH 1/3] Explicitly fake status if cancelling stream --- src/core/ext/transport/chttp2/transport/chttp2_transport.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 6d25f5c7724..46e749bc555 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -2095,6 +2095,7 @@ void grpc_chttp2_cancel_stream(grpc_chttp2_transport* t, grpc_chttp2_stream* s, } if (due_to_error != GRPC_ERROR_NONE && !s->seen_error) { s->seen_error = true; + grpc_chttp2_fake_status(t, s, GRPC_ERROR_REF(due_to_error)); } grpc_chttp2_mark_stream_closed(t, s, 1, 1, due_to_error); } From 18763aa8647a2fd3dfc7d2e7d0acc764b3bc5bcb Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Tue, 21 Jul 2020 13:00:12 -0700 Subject: [PATCH 2/3] Add comment --- src/core/ext/transport/chttp2/transport/chttp2_transport.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 46e749bc555..df648e4184d 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -2095,6 +2095,10 @@ void grpc_chttp2_cancel_stream(grpc_chttp2_transport* t, grpc_chttp2_stream* s, } if (due_to_error != GRPC_ERROR_NONE && !s->seen_error) { s->seen_error = true; + /* We are setting the fake status here instead of in + * grpc_chttp2_mark_stream_closed to handle the case where the stream is + * read and write closed, but not all callbacks have been made possibly due + * to pending data. */ grpc_chttp2_fake_status(t, s, GRPC_ERROR_REF(due_to_error)); } grpc_chttp2_mark_stream_closed(t, s, 1, 1, due_to_error); From 4c7120e1151d798a3e9f2a4110b766b74c5d5369 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Tue, 21 Jul 2020 14:15:41 -0700 Subject: [PATCH 3/3] Also fake status in grpc_chttp2_mark_stream_closed if already closed but there is an error --- .../transport/chttp2/transport/chttp2_transport.cc | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index df648e4184d..87d0aacd159 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -2095,11 +2095,6 @@ void grpc_chttp2_cancel_stream(grpc_chttp2_transport* t, grpc_chttp2_stream* s, } if (due_to_error != GRPC_ERROR_NONE && !s->seen_error) { s->seen_error = true; - /* We are setting the fake status here instead of in - * grpc_chttp2_mark_stream_closed to handle the case where the stream is - * read and write closed, but not all callbacks have been made possibly due - * to pending data. */ - grpc_chttp2_fake_status(t, s, GRPC_ERROR_REF(due_to_error)); } grpc_chttp2_mark_stream_closed(t, s, 1, 1, due_to_error); } @@ -2209,9 +2204,12 @@ void grpc_chttp2_mark_stream_closed(grpc_chttp2_transport* t, grpc_chttp2_stream* s, int close_reads, int close_writes, grpc_error* error) { if (s->read_closed && s->write_closed) { - /* already closed */ + /* already closed, but we should still fake the status if needed. */ + grpc_error* overall_error = removal_error(error, s, "Stream removed"); + if (overall_error != GRPC_ERROR_NONE) { + grpc_chttp2_fake_status(t, s, overall_error); + } grpc_chttp2_maybe_complete_recv_trailing_metadata(t, s); - GRPC_ERROR_UNREF(error); return; } bool closed_read = false;