From 75d7478d122de8142f9306463058e8d18dfd614a Mon Sep 17 00:00:00 2001
From: "Mark D. Roth" <roth@google.com>
Date: Fri, 9 Sep 2016 07:46:01 -0700
Subject: [PATCH] Fix close vs. cancel confusion in transport helper functions.

---
 src/core/lib/channel/channel_stack.c      | 10 ++++++++++
 src/core/lib/channel/channel_stack.h      |  5 +++++
 src/core/lib/channel/deadline_filter.c    | 14 +++++---------
 src/core/lib/channel/http_client_filter.c |  4 ++--
 src/core/lib/transport/transport.c        |  6 +-----
 5 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/src/core/lib/channel/channel_stack.c b/src/core/lib/channel/channel_stack.c
index fb0c70986f3..f5fa0b03907 100644
--- a/src/core/lib/channel/channel_stack.c
+++ b/src/core/lib/channel/channel_stack.c
@@ -288,3 +288,13 @@ void grpc_call_element_send_cancel_with_message(grpc_exec_ctx *exec_ctx,
                                                          optional_message);
   elem->filter->start_transport_stream_op(exec_ctx, elem, &op);
 }
+
+void grpc_call_element_send_close_with_message(grpc_exec_ctx *exec_ctx,
+                                               grpc_call_element *elem,
+                                               grpc_status_code status,
+                                               gpr_slice *optional_message) {
+  grpc_transport_stream_op op;
+  memset(&op, 0, sizeof(op));
+  grpc_transport_stream_op_add_close(&op, status, optional_message);
+  elem->filter->start_transport_stream_op(exec_ctx, elem, &op);
+}
diff --git a/src/core/lib/channel/channel_stack.h b/src/core/lib/channel/channel_stack.h
index 6b73cce380f..eeaab17d39a 100644
--- a/src/core/lib/channel/channel_stack.h
+++ b/src/core/lib/channel/channel_stack.h
@@ -290,6 +290,11 @@ void grpc_call_element_send_cancel_with_message(grpc_exec_ctx *exec_ctx,
                                                 grpc_status_code status,
                                                 gpr_slice *optional_message);
 
+void grpc_call_element_send_close_with_message(grpc_exec_ctx *exec_ctx,
+                                               grpc_call_element *cur_elem,
+                                               grpc_status_code status,
+                                               gpr_slice *optional_message);
+
 extern int grpc_trace_channel;
 
 #define GRPC_CALL_LOG_OP(sev, elem, op) \
diff --git a/src/core/lib/channel/deadline_filter.c b/src/core/lib/channel/deadline_filter.c
index 5e91d524af0..9597783b63d 100644
--- a/src/core/lib/channel/deadline_filter.c
+++ b/src/core/lib/channel/deadline_filter.c
@@ -55,15 +55,11 @@ gpr_log(GPR_INFO, "==> %s(), is_client=%d", __func__, deadline_state->is_client)
   gpr_mu_unlock(&deadline_state->timer_mu);
   if (error != GRPC_ERROR_CANCELLED) {
 gpr_log(GPR_INFO, "DEADLINE_EXCEEDED");
-// FIXME: change grpc_call_element_send_cancel_with_message to not call close
-//    grpc_call_element_send_cancel(exec_ctx, elem);
-    grpc_transport_stream_op op;
-    memset(&op, 0, sizeof(op));
-    op.cancel_error = grpc_error_set_int(
-        GRPC_ERROR_CREATE("Deadline Exceeded"),
-        GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_DEADLINE_EXCEEDED);
-    elem->filter->start_transport_stream_op(exec_ctx, elem, &op);
-    GRPC_ERROR_UNREF(op.cancel_error);
+    gpr_slice msg = gpr_slice_from_static_string("Deadline Exceeded");
+    grpc_call_element_send_cancel_with_message(exec_ctx, elem,
+                                               GRPC_STATUS_DEADLINE_EXCEEDED,
+                                               &msg);
+    gpr_slice_unref(msg);
   }
   GRPC_CALL_STACK_UNREF(exec_ctx, deadline_state->call_stack, "deadline_timer");
 }
diff --git a/src/core/lib/channel/http_client_filter.c b/src/core/lib/channel/http_client_filter.c
index edcc741ff6f..1dc05fb20d1 100644
--- a/src/core/lib/channel/http_client_filter.c
+++ b/src/core/lib/channel/http_client_filter.c
@@ -103,8 +103,8 @@ static grpc_mdelem *client_recv_filter(void *user_data, grpc_mdelem *md) {
                  grpc_mdstr_as_c_string(md->value));
     gpr_slice message = gpr_slice_from_copied_string(message_string);
     gpr_free(message_string);
-    grpc_call_element_send_cancel_with_message(a->exec_ctx, a->elem,
-                                               GRPC_STATUS_CANCELLED, &message);
+    grpc_call_element_send_close_with_message(a->exec_ctx, a->elem,
+                                              GRPC_STATUS_CANCELLED, &message);
     return NULL;
   } else if (md == GRPC_MDELEM_CONTENT_TYPE_APPLICATION_SLASH_GRPC) {
     return NULL;
diff --git a/src/core/lib/transport/transport.c b/src/core/lib/transport/transport.c
index d4e197fa5cd..205a1367422 100644
--- a/src/core/lib/transport/transport.c
+++ b/src/core/lib/transport/transport.c
@@ -220,11 +220,7 @@ void grpc_transport_stream_op_add_cancellation_with_message(
     error = GRPC_ERROR_CREATE("Call cancelled");
   }
   error = grpc_error_set_int(error, GRPC_ERROR_INT_GRPC_STATUS, status);
-  // TODO(ctiller): We are intentionally setting close_error instead of
-  // cancel_error here.  This is an ugly hack and should be replaced
-  // by a more general-purpose mechanism that allows us to control
-  // cancel/close behavior.
-  add_error(op, &op->close_error, error);
+  add_error(op, &op->cancel_error, error);
 }
 
 void grpc_transport_stream_op_add_close(grpc_transport_stream_op *op,