Merge pull request #10475 from markdroth/closure_debug_schedule_check

Add check that we don't schedule the same closure twice at once.

6 files changed, 36 insertions, 0 deletions

diff --git a/src/core/lib/iomgr/closure.c b/src/core/lib/iomgr/closure.c
index 6633fb68ec..8ef0b210ad 100644
--- a/src/core/lib/iomgr/closure.c
+++ b/src/core/lib/iomgr/closure.c
@@ -45,6 +45,9 @@ grpc_closure *grpc_closure_init(grpc_closure *closure, grpc_iomgr_cb_func cb,
   closure->cb = cb;
   closure->cb_arg = cb_arg;
   closure->scheduler = scheduler;
+#ifndef NDEBUG
+  closure->scheduled = false;
+#endif
   return closure;
 }
 
@@ -137,6 +140,10 @@ void grpc_closure_sched(grpc_exec_ctx *exec_ctx, grpc_closure *c,
                         grpc_error *error) {
   GPR_TIMER_BEGIN("grpc_closure_sched", 0);
   if (c != NULL) {
+#ifndef NDEBUG
+    GPR_ASSERT(!c->scheduled);
+    c->scheduled = true;
+#endif
     assert(c->cb);
     c->scheduler->vtable->sched(exec_ctx, c, error);
   } else {
@@ -149,6 +156,10 @@ void grpc_closure_list_sched(grpc_exec_ctx *exec_ctx, grpc_closure_list *list) {
   grpc_closure *c = list->head;
   while (c != NULL) {
     grpc_closure *next = c->next_data.next;
+#ifndef NDEBUG
+    GPR_ASSERT(!c->scheduled);
+    c->scheduled = true;
+#endif
     assert(c->cb);
     c->scheduler->vtable->sched(exec_ctx, c, c->error_data.error);
     c = next;
diff --git a/src/core/lib/iomgr/closure.h b/src/core/lib/iomgr/closure.h
index 2510d50b42..2bedbf00d6 100644
--- a/src/core/lib/iomgr/closure.h
+++ b/src/core/lib/iomgr/closure.h
@@ -99,6 +99,10 @@ struct grpc_closure {
     grpc_error *error;
     uintptr_t scratch;
   } error_data;
+
+#ifndef NDEBUG
+  bool scheduled;
+#endif
 };
 
 /** Initializes \a closure with \a cb and \a cb_arg. Returns \a closure. */
diff --git a/src/core/lib/iomgr/combiner.c b/src/core/lib/iomgr/combiner.c
index 2bc476bbef..05cdbdad2b 100644
--- a/src/core/lib/iomgr/combiner.c
+++ b/src/core/lib/iomgr/combiner.c
@@ -319,6 +319,9 @@ bool grpc_combiner_continue_exec_ctx(grpc_exec_ctx *exec_ctx) {
     GPR_TIMER_BEGIN("combiner.exec1", 0);
     grpc_closure *cl = (grpc_closure *)n;
     error_data err = unpack_error_data(cl->error_data.scratch);
+#ifndef NDEBUG
+    cl->scheduled = false;
+#endif
     cl->cb(exec_ctx, cl->cb_arg, err.error);
     if (err.covered_by_poller) {
       gpr_atm_no_barrier_fetch_add(&lock->elements_covered_by_poller, -1);
@@ -337,6 +340,9 @@ bool grpc_combiner_continue_exec_ctx(grpc_exec_ctx *exec_ctx) {
           gpr_log(GPR_DEBUG, "C:%p execute_final[%d] c=%p", lock, loops, c));
       grpc_closure *next = c->next_data.next;
       grpc_error *error = c->error_data.error;
+#ifndef NDEBUG
+      c->scheduled = false;
+#endif
       c->cb(exec_ctx, c->cb_arg, error);
       GRPC_ERROR_UNREF(error);
       c = next;
diff --git a/src/core/lib/iomgr/ev_epoll_linux.c b/src/core/lib/iomgr/ev_epoll_linux.c
index 4a0f91391f..e603a75593 100644
--- a/src/core/lib/iomgr/ev_epoll_linux.c
+++ b/src/core/lib/iomgr/ev_epoll_linux.c
@@ -1348,6 +1348,9 @@ static bool maybe_do_workqueue_work(grpc_exec_ctx *exec_ctx,
       }
       grpc_closure *c = (grpc_closure *)n;
       grpc_error *error = c->error_data.error;
+#ifndef NDEBUG
+      c->scheduled = false;
+#endif
       c->cb(exec_ctx, c->cb_arg, error);
       GRPC_ERROR_UNREF(error);
       return true;
diff --git a/src/core/lib/iomgr/exec_ctx.c b/src/core/lib/iomgr/exec_ctx.c
index 83bb436bd0..2532a708e7 100644
--- a/src/core/lib/iomgr/exec_ctx.c
+++ b/src/core/lib/iomgr/exec_ctx.c
@@ -73,6 +73,9 @@ bool grpc_exec_ctx_flush(grpc_exec_ctx *exec_ctx) {
         grpc_closure *next = c->next_data.next;
         grpc_error *error = c->error_data.error;
         did_something = true;
+#ifndef NDEBUG
+        c->scheduled = false;
+#endif
         c->cb(exec_ctx, c->cb_arg, error);
         GRPC_ERROR_UNREF(error);
         c = next;
@@ -93,6 +96,9 @@ void grpc_exec_ctx_finish(grpc_exec_ctx *exec_ctx) {
 
 static void exec_ctx_run(grpc_exec_ctx *exec_ctx, grpc_closure *closure,
                          grpc_error *error) {
+#ifndef NDEBUG
+  closure->scheduled = false;
+#endif
   closure->cb(exec_ctx, closure->cb_arg, error);
   GRPC_ERROR_UNREF(error);
 }
diff --git a/src/core/lib/iomgr/executor.c b/src/core/lib/iomgr/executor.c
index ae3e2eabc3..75fd5b1c50 100644
--- a/src/core/lib/iomgr/executor.c
+++ b/src/core/lib/iomgr/executor.c
@@ -83,6 +83,9 @@ static void closure_exec_thread_func(void *ignored) {
       while (c != NULL) {
         grpc_closure *next = c->next_data.next;
         grpc_error *error = c->error_data.error;
+#ifndef NDEBUG
+        c->scheduled = false;
+#endif
         c->cb(&exec_ctx, c->cb_arg, error);
         GRPC_ERROR_UNREF(error);
         c = next;
@@ -146,6 +149,9 @@ void grpc_executor_shutdown(grpc_exec_ctx *exec_ctx) {
   while (c != NULL) {
     grpc_closure *next = c->next_data.next;
     grpc_error *error = c->error_data.error;
+#ifndef NDEBUG
+    c->scheduled = false;
+#endif
     c->cb(exec_ctx, c->cb_arg, error);
     GRPC_ERROR_UNREF(error);
     c = next;