Fix resource leak

4 files changed, 47 insertions, 22 deletions

diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index ff883dbd76..88ae81a35d 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -564,11 +564,11 @@ static const char *write_state_name(grpc_chttp2_write_state st) {
 }
 
 static void set_write_state(grpc_chttp2_transport *t,
-                            grpc_chttp2_write_state st) {
-  GRPC_CHTTP2_IF_TRACING(gpr_log(GPR_DEBUG, "W:%p %s state %s -> %s", t,
+                            grpc_chttp2_write_state st, const char *reason) {
+  GRPC_CHTTP2_IF_TRACING(gpr_log(GPR_DEBUG, "W:%p %s state %s -> %s [%s]", t,
                                  t->is_client ? "CLIENT" : "SERVER",
                                  write_state_name(t->write_state),
-                                 write_state_name(st)));
+                                 write_state_name(st), reason));
   t->write_state = st;
 }
 
@@ -579,7 +579,7 @@ void grpc_chttp2_initiate_write(grpc_exec_ctx *exec_ctx,
 
   switch (t->write_state) {
     case GRPC_CHTTP2_WRITE_STATE_IDLE:
-      set_write_state(t, GRPC_CHTTP2_WRITE_STATE_WRITING);
+      set_write_state(t, GRPC_CHTTP2_WRITE_STATE_WRITING, reason);
       GRPC_CHTTP2_REF_TRANSPORT(t, "writing");
       grpc_combiner_execute_finally(exec_ctx, t->combiner,
                                     &t->write_action_begin_locked,
@@ -590,12 +590,14 @@ void grpc_chttp2_initiate_write(grpc_exec_ctx *exec_ctx,
           t,
           covered_by_poller
               ? GRPC_CHTTP2_WRITE_STATE_WRITING_WITH_MORE_AND_COVERED_BY_POLLER
-              : GRPC_CHTTP2_WRITE_STATE_WRITING_WITH_MORE);
+              : GRPC_CHTTP2_WRITE_STATE_WRITING_WITH_MORE,
+          reason);
       break;
     case GRPC_CHTTP2_WRITE_STATE_WRITING_WITH_MORE:
       if (covered_by_poller) {
         set_write_state(
-            t, GRPC_CHTTP2_WRITE_STATE_WRITING_WITH_MORE_AND_COVERED_BY_POLLER);
+            t, GRPC_CHTTP2_WRITE_STATE_WRITING_WITH_MORE_AND_COVERED_BY_POLLER,
+            reason);
       }
       break;
     case GRPC_CHTTP2_WRITE_STATE_WRITING_WITH_MORE_AND_COVERED_BY_POLLER:
@@ -620,10 +622,10 @@ static void write_action_begin_locked(grpc_exec_ctx *exec_ctx, void *gt,
   grpc_chttp2_transport *t = gt;
   GPR_ASSERT(t->write_state != GRPC_CHTTP2_WRITE_STATE_IDLE);
   if (!t->closed && grpc_chttp2_begin_write(exec_ctx, t)) {
-    set_write_state(t, GRPC_CHTTP2_WRITE_STATE_WRITING);
+    set_write_state(t, GRPC_CHTTP2_WRITE_STATE_WRITING, "begin writing");
     grpc_exec_ctx_sched(exec_ctx, &t->write_action, GRPC_ERROR_NONE, NULL);
   } else {
-    set_write_state(t, GRPC_CHTTP2_WRITE_STATE_IDLE);
+    set_write_state(t, GRPC_CHTTP2_WRITE_STATE_IDLE, "begin writing nothing");
     GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "writing");
   }
   GPR_TIMER_END("write_action_begin_locked", 0);
@@ -661,11 +663,12 @@ static void write_action_end_locked(grpc_exec_ctx *exec_ctx, void *tp,
       GPR_UNREACHABLE_CODE(break);
     case GRPC_CHTTP2_WRITE_STATE_WRITING:
       GPR_TIMER_MARK("state=writing", 0);
-      set_write_state(t, GRPC_CHTTP2_WRITE_STATE_IDLE);
+      set_write_state(t, GRPC_CHTTP2_WRITE_STATE_IDLE, "finish writing");
       break;
     case GRPC_CHTTP2_WRITE_STATE_WRITING_WITH_MORE:
       GPR_TIMER_MARK("state=writing_stale_no_poller", 0);
-      set_write_state(t, GRPC_CHTTP2_WRITE_STATE_WRITING);
+      set_write_state(t, GRPC_CHTTP2_WRITE_STATE_WRITING,
+                      "continue writing [!covered]");
       GRPC_CHTTP2_REF_TRANSPORT(t, "writing");
       grpc_combiner_execute_finally(exec_ctx, t->combiner,
                                     &t->write_action_begin_locked,
@@ -673,7 +676,8 @@ static void write_action_end_locked(grpc_exec_ctx *exec_ctx, void *tp,
       break;
     case GRPC_CHTTP2_WRITE_STATE_WRITING_WITH_MORE_AND_COVERED_BY_POLLER:
       GPR_TIMER_MARK("state=writing_stale_with_poller", 0);
-      set_write_state(t, GRPC_CHTTP2_WRITE_STATE_WRITING);
+      set_write_state(t, GRPC_CHTTP2_WRITE_STATE_WRITING,
+                      "continue writing [covered]");
       GRPC_CHTTP2_REF_TRANSPORT(t, "writing");
       grpc_combiner_execute_finally(exec_ctx, t->combiner,
                                     &t->write_action_begin_locked,
@@ -1887,13 +1891,17 @@ static void incoming_byte_stream_update_flow_control(grpc_exec_ctx *exec_ctx,
   max_recv_bytes += t->stream_lookahead;
   if (s->max_recv_bytes < max_recv_bytes) {
     uint32_t add_max_recv_bytes = max_recv_bytes - s->max_recv_bytes;
+    bool new_window_write_is_covered_by_poller =
+        s->max_recv_bytes < have_already;
     GRPC_CHTTP2_FLOW_CREDIT_STREAM("op", t, s, max_recv_bytes,
                                    add_max_recv_bytes);
     GRPC_CHTTP2_FLOW_CREDIT_STREAM("op", t, s, incoming_window,
                                    add_max_recv_bytes);
     GRPC_CHTTP2_FLOW_CREDIT_STREAM("op", t, s, announce_window,
                                    add_max_recv_bytes);
-    grpc_chttp2_become_writable(exec_ctx, t, s, true, "read_incoming_stream");
+    grpc_chttp2_become_writable(exec_ctx, t, s,
+                                new_window_write_is_covered_by_poller,
+                                "read_incoming_stream");
   }
 }
 
diff --git a/src/core/lib/iomgr/combiner.c b/src/core/lib/iomgr/combiner.c
index 2b68240a15..48806abc38 100644
--- a/src/core/lib/iomgr/combiner.c
+++ b/src/core/lib/iomgr/combiner.c
@@ -142,11 +142,11 @@ static void push_first_on_exec_ctx(grpc_exec_ctx *exec_ctx,
 void grpc_combiner_execute(grpc_exec_ctx *exec_ctx, grpc_combiner *lock,
                            grpc_closure *cl, grpc_error *error,
                            bool covered_by_poller) {
-  GRPC_COMBINER_TRACE(gpr_log(GPR_DEBUG,
-                              "C:%p grpc_combiner_execute c=%p cov=%d", lock,
-                              cl, covered_by_poller));
   GPR_TIMER_BEGIN("combiner.execute", 0);
   gpr_atm last = gpr_atm_full_fetch_add(&lock->state, 2);
+  GRPC_COMBINER_TRACE(gpr_log(
+      GPR_DEBUG, "C:%p grpc_combiner_execute c=%p cov=%d last=%" PRIdPTR, lock,
+      cl, covered_by_poller, last));
   GPR_ASSERT(last & 1);  // ensure lock has not been destroyed
   cl->error_data.scratch =
       pack_error_data((error_data){error, covered_by_poller});
@@ -177,6 +177,8 @@ static void offload(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
 
 static void queue_offload(grpc_exec_ctx *exec_ctx, grpc_combiner *lock) {
   move_next(exec_ctx);
+  GRPC_COMBINER_TRACE(gpr_log(GPR_DEBUG, "C:%p queue_offload --> %p", lock,
+                              lock->optional_workqueue));
   grpc_workqueue_enqueue(exec_ctx, lock->optional_workqueue, &lock->offload,
                          GRPC_ERROR_NONE);
 }
@@ -189,6 +191,15 @@ bool grpc_combiner_continue_exec_ctx(grpc_exec_ctx *exec_ctx) {
     return false;
   }
 
+  GRPC_COMBINER_TRACE(
+      gpr_log(GPR_DEBUG,
+              "C:%p grpc_combiner_continue_exec_ctx workqueue=%p "
+              "is_covered_by_poller=%d exec_ctx_ready_to_finish=%d "
+              "time_to_execute_final_list=%d",
+              lock, lock->optional_workqueue, is_covered_by_poller(lock),
+              grpc_exec_ctx_ready_to_finish(exec_ctx),
+              lock->time_to_execute_final_list));
+
   if (lock->optional_workqueue != NULL && is_covered_by_poller(lock) &&
       grpc_exec_ctx_ready_to_finish(exec_ctx)) {
     GPR_TIMER_MARK("offload_from_finished_exec_ctx", 0);
@@ -289,9 +300,9 @@ static void enqueue_finally(grpc_exec_ctx *exec_ctx, void *closure,
 void grpc_combiner_execute_finally(grpc_exec_ctx *exec_ctx, grpc_combiner *lock,
                                    grpc_closure *closure, grpc_error *error,
                                    bool covered_by_poller) {
-  GRPC_COMBINER_TRACE(gpr_log(GPR_DEBUG,
-                              "C:%p grpc_combiner_execute_finally c=%p; ac=%p",
-                              lock, closure, exec_ctx->active_combiner));
+  GRPC_COMBINER_TRACE(gpr_log(
+      GPR_DEBUG, "C:%p grpc_combiner_execute_finally c=%p; ac=%p; cov=%d", lock,
+      closure, exec_ctx->active_combiner, covered_by_poller));
   GPR_TIMER_BEGIN("combiner.execute_finally", 0);
   if (exec_ctx->active_combiner != lock) {
     GPR_TIMER_MARK("slowpath", 0);
diff --git a/src/core/lib/iomgr/iomgr.c b/src/core/lib/iomgr/iomgr.c
index d67d388b8c..4fd83e0b22 100644
--- a/src/core/lib/iomgr/iomgr.c
+++ b/src/core/lib/iomgr/iomgr.c
@@ -112,6 +112,14 @@ void grpc_iomgr_shutdown(void) {
       continue;
     }
     if (g_root_object.next != &g_root_object) {
+      if (grpc_iomgr_abort_on_leaks()) {
+        gpr_log(GPR_DEBUG, "Failed to free %" PRIuPTR
+                           " iomgr objects before shutdown deadline: "
+                           "memory leaks are likely",
+                count_objects());
+        dump_objects("LEAKED");
+        abort();
+      }
       gpr_timespec short_deadline = gpr_time_add(
           gpr_now(GPR_CLOCK_REALTIME), gpr_time_from_millis(100, GPR_TIMESPAN));
       if (gpr_cv_wait(&g_rcv, &g_mu, short_deadline)) {
@@ -122,9 +130,6 @@ void grpc_iomgr_shutdown(void) {
                                "memory leaks are likely",
                     count_objects());
             dump_objects("LEAKED");
-            if (grpc_iomgr_abort_on_leaks()) {
-              abort();
-            }
           }
           break;
         }
diff --git a/src/core/lib/support/log.c b/src/core/lib/support/log.c
index 6fbd947f4b..af1651dae5 100644
--- a/src/core/lib/support/log.c
+++ b/src/core/lib/support/log.c
@@ -60,8 +60,9 @@ const char *gpr_log_severity_string(gpr_log_severity severity) {
 
 void gpr_log_message(const char *file, int line, gpr_log_severity severity,
                      const char *message) {
-  if ((gpr_atm)severity < gpr_atm_no_barrier_load(&g_min_severity_to_print))
+  if ((gpr_atm)severity < gpr_atm_no_barrier_load(&g_min_severity_to_print)) {
     return;
+  }
 
   gpr_log_func_args lfargs;
   memset(&lfargs, 0, sizeof(lfargs));