Fix refcounting bug

4 files changed, 19 insertions, 14 deletions

diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index 51191f338e..84357b8075 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -1015,7 +1015,7 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx, void *stream_op,
   }
 
   if (op->cancel_error != GRPC_ERROR_NONE) {
-    grpc_chttp2_cancel_stream(exec_ctx, t, s, GRPC_ERROR_REF(op->cancel_error));
+    grpc_chttp2_cancel_stream(exec_ctx, t, s, op->cancel_error);
   }
 
   if (op->send_initial_metadata != NULL) {
@@ -1066,8 +1066,9 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx, void *stream_op,
         s->send_initial_metadata = NULL;
         grpc_chttp2_complete_closure_step(
             exec_ctx, t, s, &s->send_initial_metadata_finished,
-            GRPC_ERROR_CREATE(
-                "Attempt to send initial metadata after stream was closed"),
+            GRPC_ERROR_CREATE_REFERENCING(
+                "Attempt to send initial metadata after stream was closed",
+                &s->write_closed_error, 1),
             "send_initial_metadata_finished");
       }
     }
@@ -1562,11 +1563,6 @@ void grpc_chttp2_mark_stream_closed(grpc_exec_ctx *exec_ctx,
   if (close_reads && !s->read_closed) {
     s->read_closed_error = GRPC_ERROR_REF(error);
     s->read_closed = true;
-    for (int i = 0; i < 2; i++) {
-      if (s->published_metadata[i] == GRPC_METADATA_NOT_PUBLISHED) {
-        s->published_metadata[i] = GPRC_METADATA_PUBLISHED_AT_CLOSE;
-      }
-    }
     closed_read = true;
   }
   if (close_writes && !s->write_closed) {
@@ -1589,6 +1585,11 @@ void grpc_chttp2_mark_stream_closed(grpc_exec_ctx *exec_ctx,
     }
   }
   if (closed_read) {
+    for (int i = 0; i < 2; i++) {
+      if (s->published_metadata[i] == GRPC_METADATA_NOT_PUBLISHED) {
+        s->published_metadata[i] = GPRC_METADATA_PUBLISHED_AT_CLOSE;
+      }
+    }
     decrement_active_streams_locked(exec_ctx, t, s);
     grpc_chttp2_maybe_complete_recv_initial_metadata(exec_ctx, t, s);
     grpc_chttp2_maybe_complete_recv_message(exec_ctx, t, s);
diff --git a/src/core/lib/iomgr/error.c b/src/core/lib/iomgr/error.c
index ceff429fcc..dbe5b139f9 100644
--- a/src/core/lib/iomgr/error.c
+++ b/src/core/lib/iomgr/error.c
@@ -170,7 +170,7 @@ bool grpc_error_is_special(grpc_error *err) {
 #ifdef GRPC_ERROR_REFCOUNT_DEBUG
 grpc_error *grpc_error_ref(grpc_error *err, const char *file, int line,
                            const char *func) {
-  if (is_special(err)) return err;
+  if (grpc_error_is_special(err)) return err;
   gpr_log(GPR_DEBUG, "%p: %" PRIdPTR " -> %" PRIdPTR " [%s:%d %s]", err,
           err->refs.count, err->refs.count + 1, file, line, func);
   gpr_ref(&err->refs);
@@ -197,7 +197,7 @@ static void error_destroy(grpc_error *err) {
 #ifdef GRPC_ERROR_REFCOUNT_DEBUG
 void grpc_error_unref(grpc_error *err, const char *file, int line,
                       const char *func) {
-  if (is_special(err)) return;
+  if (grpc_error_is_special(err)) return;
   gpr_log(GPR_DEBUG, "%p: %" PRIdPTR " -> %" PRIdPTR " [%s:%d %s]", err,
           err->refs.count, err->refs.count - 1, file, line, func);
   if (gpr_unref(&err->refs)) {
diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h
index ffacdac393..23e85f20b3 100644
--- a/src/core/lib/iomgr/error.h
+++ b/src/core/lib/iomgr/error.h
@@ -165,7 +165,7 @@ grpc_error *grpc_error_create(const char *file, int line, const char *desc,
 #define GRPC_ERROR_CREATE_REFERENCING(desc, errs, count) \
   grpc_error_create(__FILE__, __LINE__, desc, errs, count)
 
-//#define GRPC_ERROR_REFCOUNT_DEBUG
+#define GRPC_ERROR_REFCOUNT_DEBUG
 #ifdef GRPC_ERROR_REFCOUNT_DEBUG
 grpc_error *grpc_error_ref(grpc_error *err, const char *file, int line,
                            const char *func);
diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c
index f53e940fde..fde3430388 100644
--- a/src/core/lib/surface/call.c
+++ b/src/core/lib/surface/call.c
@@ -536,7 +536,6 @@ static void done_termination(grpc_exec_ctx *exec_ctx, void *tcp,
                              grpc_error *error) {
   termination_closure *tc = tcp;
   GRPC_CALL_INTERNAL_UNREF(exec_ctx, tc->call, "termination");
-  GRPC_ERROR_UNREF(tc->error);
   gpr_free(tc);
 }
 
@@ -966,9 +965,14 @@ static grpc_error *consolidate_batch_errors(batch_control *bctl) {
   if (n == 0) {
     return GRPC_ERROR_NONE;
   } else if (n == 1) {
-    return GRPC_ERROR_REF(bctl->errors[0]);
+    return bctl->errors[0];
   } else {
-    return GRPC_ERROR_CREATE_REFERENCING("Call batch failed", bctl->errors, n);
+    grpc_error *error =
+        GRPC_ERROR_CREATE_REFERENCING("Call batch failed", bctl->errors, n);
+    for (size_t i = 0; i < n; i++) {
+      GRPC_ERROR_UNREF(bctl->errors[i]);
+    }
+    return error;
   }
 }