Review feedback

include/grpc/impl/codegen/grpc_types.h

@@ -34,10 +34,10 @@
 #ifndef GRPC_IMPL_CODEGEN_GRPC_TYPES_H
 #define GRPC_IMPL_CODEGEN_GRPC_TYPES_H
 
+#include <grpc/impl/codegen/compression_types.h>
+#include <grpc/impl/codegen/exec_ctx_fwd.h>
 #include <grpc/impl/codegen/gpr_types.h>
 #include <grpc/impl/codegen/slice.h>
-
-#include <grpc/impl/codegen/compression_types.h>
 #include <grpc/impl/codegen/status.h>
 
 #include <stddef.h>

src/core/lib/iomgr/executor.h

@@ -48,6 +48,6 @@ void grpc_executor_init();
 void grpc_executor_push(grpc_closure *closure, grpc_error *error);
 
 /** Shutdown the executor, running all pending work as part of the call */
-void grpc_executor_shutdown();
+void grpc_executor_shutdown(grpc_exec_ctx *exec_ctx);
 
 #endif /* GRPC_CORE_LIB_IOMGR_EXECUTOR_H */

src/core/lib/iomgr/iomgr.c

@@ -83,11 +83,10 @@ static void dump_objects(const char *kind) {
   }
 }
 
-void grpc_iomgr_shutdown(void) {
+void grpc_iomgr_shutdown(grpc_exec_ctx *exec_ctx) {
   gpr_timespec shutdown_deadline = gpr_time_add(
       gpr_now(GPR_CLOCK_REALTIME), gpr_time_from_seconds(10, GPR_TIMESPAN));
   gpr_timespec last_warning_time = gpr_now(GPR_CLOCK_REALTIME);
-  grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
 
   grpc_iomgr_platform_flush();
 
@@ -104,10 +103,9 @@ void grpc_iomgr_shutdown(void) {
       }
       last_warning_time = gpr_now(GPR_CLOCK_REALTIME);
     }
-    if (grpc_timer_check(&exec_ctx, gpr_inf_future(GPR_CLOCK_MONOTONIC),
-                         NULL)) {
+    if (grpc_timer_check(exec_ctx, gpr_inf_future(GPR_CLOCK_MONOTONIC), NULL)) {
       gpr_mu_unlock(&g_mu);
-      grpc_exec_ctx_flush(&exec_ctx);
+      grpc_exec_ctx_flush(exec_ctx);
       gpr_mu_lock(&g_mu);
       continue;
     }
@@ -138,8 +136,8 @@ void grpc_iomgr_shutdown(void) {
   }
   gpr_mu_unlock(&g_mu);
 
-  grpc_timer_list_shutdown(&exec_ctx);
-  grpc_exec_ctx_finish(&exec_ctx);
+  grpc_timer_list_shutdown(exec_ctx);
+  grpc_exec_ctx_flush(exec_ctx);
 
   /* ensure all threads have left g_mu */
   gpr_mu_lock(&g_mu);

src/core/lib/iomgr/iomgr.h

@@ -34,12 +34,14 @@
 #ifndef GRPC_CORE_LIB_IOMGR_IOMGR_H
 #define GRPC_CORE_LIB_IOMGR_IOMGR_H
 
+#include <grpc/impl/codegen/exec_ctx_fwd.h>
 #include "src/core/lib/iomgr/port.h"
 
 /** Initializes the iomgr. */
 void grpc_iomgr_init(void);
 
-/** Signals the intention to shutdown the iomgr. */
-void grpc_iomgr_shutdown(void);
+/** Signals the intention to shutdown the iomgr. Expects to be able to flush
+ * exec_ctx. */
+void grpc_iomgr_shutdown(grpc_exec_ctx *exec_ctx);
 
 #endif /* GRPC_CORE_LIB_IOMGR_IOMGR_H */

src/core/lib/security/credentials/jwt/jwt_verifier.c

@@ -629,7 +629,7 @@ static void on_keys_retrieved(grpc_exec_ctx *exec_ctx, void *user_data,
 end:
   if (json != NULL) grpc_json_destroy(json);
   if (verification_key != NULL) EVP_PKEY_free(verification_key);
-  ctx->user_cb(ctx->user_data, status, claims);
+  ctx->user_cb(exec_ctx, ctx->user_data, status, claims);
   verifier_cb_ctx_destroy(exec_ctx, ctx);
 }
 
@@ -682,7 +682,8 @@ static void on_openid_config_retrieved(grpc_exec_ctx *exec_ctx, void *user_data,
 
 error:
   if (json != NULL) grpc_json_destroy(json);
-  ctx->user_cb(ctx->user_data, GRPC_JWT_VERIFIER_KEY_RETRIEVAL_ERROR, NULL);
+  ctx->user_cb(exec_ctx, ctx->user_data, GRPC_JWT_VERIFIER_KEY_RETRIEVAL_ERROR,
+               NULL);
   verifier_cb_ctx_destroy(exec_ctx, ctx);
 }
 
@@ -793,7 +794,8 @@ static void retrieve_key_and_verify(grpc_exec_ctx *exec_ctx,
   return;
 
 error:
-  ctx->user_cb(ctx->user_data, GRPC_JWT_VERIFIER_KEY_RETRIEVAL_ERROR, NULL);
+  ctx->user_cb(exec_ctx, ctx->user_data, GRPC_JWT_VERIFIER_KEY_RETRIEVAL_ERROR,
+               NULL);
   verifier_cb_ctx_destroy(exec_ctx, ctx);
 }
 
@@ -844,7 +846,7 @@ void grpc_jwt_verifier_verify(grpc_exec_ctx *exec_ctx,
 error:
   if (header != NULL) jose_header_destroy(exec_ctx, header);
   if (claims != NULL) grpc_jwt_claims_destroy(exec_ctx, claims);
-  cb(user_data, GRPC_JWT_VERIFIER_BAD_FORMAT, NULL);
+  cb(exec_ctx, user_data, GRPC_JWT_VERIFIER_BAD_FORMAT, NULL);
 }
 
 grpc_jwt_verifier *grpc_jwt_verifier_create(

src/core/lib/security/credentials/jwt/jwt_verifier.h

@@ -115,7 +115,8 @@ void grpc_jwt_verifier_destroy(grpc_jwt_verifier *verifier);
    is done (maybe in another thread).
    It is the responsibility of the callee to call grpc_jwt_claims_destroy on
    the claims. */
-typedef void (*grpc_jwt_verification_done_cb)(void *user_data,
+typedef void (*grpc_jwt_verification_done_cb)(grpc_exec_ctx *exec_ctx,
+                                              void *user_data,
                                               grpc_jwt_verifier_status status,
                                               grpc_jwt_claims *claims);
 

src/core/lib/slice/slice_buffer.c

@@ -75,10 +75,7 @@ void grpc_slice_buffer_destroy_internal(grpc_exec_ctx *exec_ctx,
 
 void grpc_slice_buffer_destroy(grpc_slice_buffer *sb) {
   grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
-  grpc_slice_buffer_reset_and_unref_internal(&exec_ctx, sb);
-  if (sb->slices != sb->inlined) {
-    gpr_free(sb->slices);
-  }
+  grpc_slice_buffer_destroy_internal(&exec_ctx, sb);
   grpc_exec_ctx_finish(&exec_ctx);
 }
 

src/core/lib/surface/init.c

@@ -227,9 +227,9 @@ void grpc_shutdown(void) {
   grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
   gpr_mu_lock(&g_init_mu);
   if (--g_initializations == 0) {
-    grpc_executor_shutdown();
+    grpc_executor_shutdown(&exec_ctx);
     grpc_cq_global_shutdown();
-    grpc_iomgr_shutdown();
+    grpc_iomgr_shutdown(&exec_ctx);
     gpr_timers_global_destroy();
     grpc_tracer_shutdown();
     for (i = g_number_of_plugins; i >= 0; i--) {

test/core/internal_api_canaries/iomgr.c

@@ -48,7 +48,7 @@
 static void test_code(void) {
   /* iomgr.h */
   grpc_iomgr_init();
-  grpc_iomgr_shutdown();
+  grpc_iomgr_shutdown(NULL);
 
   /* closure.h */
   grpc_closure closure;
@@ -99,7 +99,7 @@ static void test_code(void) {
   /* executor.h */
   grpc_executor_init();
   grpc_executor_push(&closure, GRPC_ERROR_CREATE("Phi"));
-  grpc_executor_shutdown();
+  grpc_executor_shutdown(NULL);
 
   /* pollset.h */
   grpc_pollset_size();

test/core/iomgr/fd_conservation_posix_test.c

@@ -65,6 +65,10 @@ int main(int argc, char **argv) {
 
   grpc_resource_quota_unref(resource_quota);
 
-  grpc_iomgr_shutdown();
+  {
+    grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
+    grpc_iomgr_shutdown(&exec_ctx);
+    grpc_exec_ctx_finish(&exec_ctx);
+  }
   return 0;
 }

test/core/iomgr/fd_posix_test.c

@@ -548,9 +548,10 @@ int main(int argc, char **argv) {
   test_grpc_fd_change();
   grpc_closure_init(&destroyed, destroy_pollset, g_pollset);
   grpc_pollset_shutdown(&exec_ctx, g_pollset, &destroyed);
-  grpc_exec_ctx_finish(&exec_ctx);
+  grpc_exec_ctx_flush(&exec_ctx);
   gpr_free(g_pollset);
-  grpc_iomgr_shutdown();
+  grpc_iomgr_shutdown(&exec_ctx);
+  grpc_exec_ctx_finish(&exec_ctx);
   return 0;
 }
 

test/core/iomgr/resolve_address_test.c

@@ -172,7 +172,11 @@ int main(int argc, char **argv) {
   test_ipv6_without_port();
   test_invalid_ip_addresses();
   test_unparseable_hostports();
-  grpc_iomgr_shutdown();
-  grpc_executor_shutdown();
+  {
+    grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
+    grpc_executor_shutdown(&exec_ctx);
+    grpc_iomgr_shutdown(&exec_ctx);
+    grpc_exec_ctx_finish(&exec_ctx);
+  }
   return 0;
 }

test/core/iomgr/udp_server_test.c

@@ -238,7 +238,7 @@ int main(int argc, char **argv) {
   grpc_pollset_shutdown(&exec_ctx, g_pollset, &destroyed);
   grpc_exec_ctx_finish(&exec_ctx);
   gpr_free(g_pollset);
-  grpc_iomgr_shutdown();
+  grpc_shutdown();
   return 0;
 }
 

test/core/security/jwt_verifier_test.c

@@ -306,16 +306,14 @@ static int httpcli_get_google_keys_for_email(
   return 1;
 }
 
-static void on_verification_success(void *user_data,
+static void on_verification_success(grpc_exec_ctx *exec_ctx, void *user_data,
                                     grpc_jwt_verifier_status status,
                                     grpc_jwt_claims *claims) {
   GPR_ASSERT(status == GRPC_JWT_VERIFIER_OK);
   GPR_ASSERT(claims != NULL);
   GPR_ASSERT(user_data == (void *)expected_user_data);
   GPR_ASSERT(strcmp(grpc_jwt_claims_audience(claims), expected_audience) == 0);
-  grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
-  grpc_jwt_claims_destroy(&exec_ctx, claims);
-  grpc_exec_ctx_finish(&exec_ctx);
+  grpc_jwt_claims_destroy(exec_ctx, claims);
 }
 
 static void test_jwt_verifier_google_email_issuer_success(void) {
@@ -423,7 +421,8 @@ static void test_jwt_verifier_url_issuer_success(void) {
   grpc_httpcli_set_override(NULL, NULL);
 }
 
-static void on_verification_key_retrieval_error(void *user_data,
+static void on_verification_key_retrieval_error(grpc_exec_ctx *exec_ctx,
+                                                void *user_data,
                                                 grpc_jwt_verifier_status status,
                                                 grpc_jwt_claims *claims) {
   GPR_ASSERT(status == GRPC_JWT_VERIFIER_KEY_RETRIEVAL_ERROR);
@@ -508,7 +507,8 @@ static void corrupt_jwt_sig(char *jwt) {
   grpc_slice_unref(sig);
 }
 
-static void on_verification_bad_signature(void *user_data,
+static void on_verification_bad_signature(grpc_exec_ctx *exec_ctx,
+                                          void *user_data,
                                           grpc_jwt_verifier_status status,
                                           grpc_jwt_claims *claims) {
   GPR_ASSERT(status == GRPC_JWT_VERIFIER_BAD_SIGNATURE);
@@ -549,7 +549,7 @@ static int httpcli_get_should_not_be_called(grpc_exec_ctx *exec_ctx,
   return 1;
 }
 
-static void on_verification_bad_format(void *user_data,
+static void on_verification_bad_format(grpc_exec_ctx *exec_ctx, void *user_data,
                                        grpc_jwt_verifier_status status,
                                        grpc_jwt_claims *claims) {
   GPR_ASSERT(status == GRPC_JWT_VERIFIER_BAD_FORMAT);

test/core/security/verify_jwt.c

@@ -59,7 +59,7 @@ static void print_usage_and_exit(gpr_cmdline *cl, const char *argv0) {
   exit(1);
 }
 
-static void on_jwt_verification_done(void *user_data,
+static void on_jwt_verification_done(grpc_exec_ctx *exec_ctx, void *user_data,
                                      grpc_jwt_verifier_status status,
                                      grpc_jwt_claims *claims) {
   synchronizer *sync = user_data;
@@ -72,7 +72,7 @@ static void on_jwt_verification_done(void *user_data,
         grpc_json_dump_to_string((grpc_json *)grpc_jwt_claims_json(claims), 2);
     printf("Claims: \n\n%s\n", claims_str);
     gpr_free(claims_str);
-    grpc_jwt_claims_destroy(claims);
+    grpc_jwt_claims_destroy(exec_ctx, claims);
   } else {
     GPR_ASSERT(claims == NULL);
     fprintf(stderr, "Verification failed with error %s\n",

tools/run_tests/sanity/core_banned_functions.py

@@ -7,12 +7,12 @@ os.chdir(os.path.join(os.path.dirname(sys.argv[0]), '../../..'))
 
 # map of banned function signature to whitelist
 BANNED_EXCEPT = {
-    'grpc_resource_quota_ref(': ('src/core/lib/iomgr/resource_quota.c'),
-    'grpc_resource_quota_unref(': ('src/core/lib/iomgr/resource_quota.c'),
-    'grpc_slice_buffer_destroy(': ('src/core/lib/slice/slice_buffer.c'),
-    'grpc_slice_buffer_reset_and_unref(': ('src/core/lib/slice/slice_buffer.c'),
-    'grpc_slice_ref(': ('src/core/lib/slice/slice.c'),
-    'grpc_slice_unref(': ('src/core/lib/slice/slice.c'),
+    'grpc_resource_quota_ref(': ['src/core/lib/iomgr/resource_quota.c'],
+    'grpc_resource_quota_unref(': ['src/core/lib/iomgr/resource_quota.c'],
+    'grpc_slice_buffer_destroy(': ['src/core/lib/slice/slice_buffer.c'],
+    'grpc_slice_buffer_reset_and_unref(': ['src/core/lib/slice/slice_buffer.c'],
+    'grpc_slice_ref(': ['src/core/lib/slice/slice.c'],
+    'grpc_slice_unref(': ['src/core/lib/slice/slice.c'],
 }
 
 errors = 0
@@ -29,4 +29,3 @@ for root, dirs, files in os.walk('src/core'):
         errors += 1
 
 assert errors == 0
-