From ff17b050e6ab499eeaf292e316d6a481b2095b3f Mon Sep 17 00:00:00 2001
From: Vijay Pai <vpai@google.com>
Date: Tue, 27 Jun 2017 11:05:29 -0700
Subject: [PATCH] Document error unref contract for iomgr cb's and
 transport/stream errors

---
 src/core/lib/iomgr/closure.h       |  4 +++-
 src/core/lib/transport/transport.c |  5 +++++
 src/core/lib/transport/transport.h | 10 ++++++++--
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/core/lib/iomgr/closure.h b/src/core/lib/iomgr/closure.h
index 2ec6f77e761..2560bf4527e 100644
--- a/src/core/lib/iomgr/closure.h
+++ b/src/core/lib/iomgr/closure.h
@@ -42,7 +42,9 @@ typedef struct grpc_closure_list {
  *
  * \param arg Arbitrary input.
  * \param error GRPC_ERROR_NONE if no error occurred, otherwise some grpc_error
- *              describing what went wrong */
+ *              describing what went wrong.
+ *              Error contract: it is not the cb's job to unref this error;
+ *              the closure scheduler will do that after the cb returns */
 typedef void (*grpc_iomgr_cb_func)(grpc_exec_ctx *exec_ctx, void *arg,
                                    grpc_error *error);
 
diff --git a/src/core/lib/transport/transport.c b/src/core/lib/transport/transport.c
index 52725846e66..6a9eba110d1 100644
--- a/src/core/lib/transport/transport.c
+++ b/src/core/lib/transport/transport.c
@@ -206,6 +206,11 @@ grpc_endpoint *grpc_transport_get_endpoint(grpc_exec_ctx *exec_ctx,
   return transport->vtable->get_endpoint(exec_ctx, transport);
 }
 
+// grpc_transport_stream_op_batch_finish_with_failure
+// is a function that must always unref cancel_error
+// though it lives in lib, it handles transport stream ops sure
+// it's grpc_transport_stream_op_batch_finish_with_failure
+
 void grpc_transport_stream_op_batch_finish_with_failure(
     grpc_exec_ctx *exec_ctx, grpc_transport_stream_op_batch *op,
     grpc_error *error) {
diff --git a/src/core/lib/transport/transport.h b/src/core/lib/transport/transport.h
index 2a1a24db203..84e53e683ac 100644
--- a/src/core/lib/transport/transport.h
+++ b/src/core/lib/transport/transport.h
@@ -198,6 +198,8 @@ struct grpc_transport_stream_op_batch_payload {
         grpc_chttp2_grpc_status_to_http2_error. Send a RST_STREAM with this
         error. */
   struct {
+    // Error contract: the transport that gets this op must cause cancel_error
+    //                 to be unref'ed after processing it
     grpc_error *cancel_error;
   } cancel_stream;
 
@@ -212,9 +214,13 @@ typedef struct grpc_transport_op {
   /** connectivity monitoring - set connectivity_state to NULL to unsubscribe */
   grpc_closure *on_connectivity_state_change;
   grpc_connectivity_state *connectivity_state;
-  /** should the transport be disconnected */
+  /** should the transport be disconnected
+   * Error contract: the transport that gets this op must cause
+   *                 disconnect_with_error to be unref'ed after processing it */
   grpc_error *disconnect_with_error;
-  /** what should the goaway contain? */
+  /** what should the goaway contain?
+   * Error contract: the transport that gets this op must cause
+   *                 goaway_error to be unref'ed after processing it */
   grpc_error *goaway_error;
   /** set the callback for accepting new streams;
       this is a permanent callback, unlike the other one-shot closures.