Other comments

include/grpc/grpc.h

@@ -75,10 +75,9 @@ GRPCAPI void grpc_init(void);
 
     The last call to grpc_shutdown will initiate cleaning up of grpc library
     internals, which can happen in another thread. Once the clean-up is done,
-    no memory is used by grpc after this call returns, nor are any instructions
+    no memory is used by grpc, nor are any instructions executing within the
-    executing within the grpc library.
+    grpc library.  Prior to calling, all application owned grpc objects must
-    Prior to calling, all application owned grpc objects must have been
+    have been destroyed. */
-    destroyed. */
 GRPCAPI void grpc_shutdown(void);
 
 /** EXPERIMENTAL. Returns 1 if the grpc library has been initialized.

src/core/lib/gprpp/thd.h

@@ -49,23 +49,16 @@ class Thread {
  public:
   class Options {
    public:
-    Options() : joinable_(true), tracked_(true) {}
+    Options() : joinable_(true) {}
+    /// Set whether the thread is joinable or detached.
     Options& set_joinable(bool joinable) {
       joinable_ = joinable;
       return *this;
     }
-    Options& set_tracked(bool tracked) {
-      tracked_ = tracked;
-      return *this;
-    }
     bool joinable() const { return joinable_; }
-    bool tracked() const { return tracked_; }
 
    private:
     bool joinable_;
-    // Whether this thread is tracked by grpc internals. Should be true for most
-    // of threads.
-    bool tracked_;
   };
   /// Default constructor only to allow use in structs that lack constructors
   /// Does not produce a validly-constructed thread; must later

src/core/lib/gprpp/thd_posix.cc

@@ -45,11 +45,9 @@ struct thd_arg {
   void* arg;               /* argument to a thread */
   const char* name;        /* name of thread. Can be nullptr. */
   bool joinable;
-  bool tracked;
 };
 
-class ThreadInternalsPosix
+class ThreadInternalsPosix : public internal::ThreadInternalsInterface {
-    : public grpc_core::internal::ThreadInternalsInterface {
  public:
   ThreadInternalsPosix(const char* thd_name, void (*thd_body)(void* arg),
                        void* arg, bool* success, const Thread::Options& options)
@@ -66,10 +64,7 @@ class ThreadInternalsPosix
     info->arg = arg;
     info->name = thd_name;
     info->joinable = options.joinable();
-    info->tracked = options.tracked();
+    Fork::IncThreadCount();
-    if (options.tracked()) {
-      grpc_core::Fork::IncThreadCount();
-    }
 
     GPR_ASSERT(pthread_attr_init(&attr) == 0);
     if (options.joinable()) {
@@ -109,13 +104,11 @@ class ThreadInternalsPosix
                           gpr_mu_unlock(&arg.thread->mu_);
 
                           if (!arg.joinable) {
-                            grpc_core::Delete(arg.thread);
+                            Delete(arg.thread);
                           }
 
                           (*arg.body)(arg.arg);
-                          if (arg.tracked) {
+                          Fork::DecThreadCount();
-                            grpc_core::Fork::DecThreadCount();
-                          }
                           return nullptr;
                         },
                         info) == 0);
@@ -125,9 +118,7 @@ class ThreadInternalsPosix
     if (!(*success)) {
       /* don't use gpr_free, as this was allocated using malloc (see above) */
       free(info);
-      if (options.tracked()) {
+      Fork::DecThreadCount();
-        grpc_core::Fork::DecThreadCount();
-      }
     }
   }
 
@@ -158,13 +149,12 @@ Thread::Thread(const char* thd_name, void (*thd_body)(void* arg), void* arg,
                bool* success, const Options& options)
     : options_(options) {
   bool outcome = false;
-  impl_ = grpc_core::New<ThreadInternalsPosix>(thd_name, thd_body, arg,
+  impl_ = New<ThreadInternalsPosix>(thd_name, thd_body, arg, &outcome, options);
-                                               &outcome, options);
   if (outcome) {
     state_ = ALIVE;
   } else {
     state_ = FAILED;
-    grpc_core::Delete(impl_);
+    Delete(impl_);
     impl_ = nullptr;
   }
 

src/core/lib/gprpp/thd_windows.cc

@@ -41,6 +41,7 @@
 #error "Unknown compiler - please file a bug report"
 #endif
 
+namespace grpc_core {
 namespace {
 class ThreadInternalsWindows;
 struct thd_info {
@@ -53,11 +54,10 @@ struct thd_info {
 
 thread_local struct thd_info* g_thd_info;
 
-class ThreadInternalsWindows
+class ThreadInternalsWindows : public internal::ThreadInternalsInterface {
-    : public grpc_core::internal::ThreadInternalsInterface {
  public:
   ThreadInternalsWindows(void (*thd_body)(void* arg), void* arg, bool* success,
-                         const grpc_core::Thread::Options& options)
+                         const Thread::Options& options)
       : started_(false) {
     gpr_mu_init(&mu_);
     gpr_cv_init(&ready_);
@@ -88,7 +88,7 @@ class ThreadInternalsWindows
                             }
                             gpr_mu_unlock(&g_thd_info->thread->mu_);
                             if (!g_thd_info->joinable) {
-                              grpc_core::Delete(g_thd_info->thread);
+                              Delete(g_thd_info->thread);
                               g_thd_info->thread = nullptr;
                             }
                             g_thd_info->body(g_thd_info->arg);
@@ -144,19 +144,16 @@ class ThreadInternalsWindows
 
 }  // namespace
 
-namespace grpc_core {
-
 Thread::Thread(const char* thd_name, void (*thd_body)(void* arg), void* arg,
                bool* success, const Options& options)
     : options_(options) {
   bool outcome = false;
-  impl_ =
+  impl_ = New<ThreadInternalsWindows>(thd_body, arg, &outcome, options);
-      grpc_core::New<ThreadInternalsWindows>(thd_body, arg, &outcome, options);
   if (outcome) {
     state_ = ALIVE;
   } else {
     state_ = FAILED;
-    grpc_core::Delete(impl_);
+    Delete(impl_);
     impl_ = nullptr;
   }
 

src/core/lib/surface/init.cc

@@ -33,6 +33,7 @@
 #include "src/core/lib/debug/stats.h"
 #include "src/core/lib/debug/trace.h"
 #include "src/core/lib/gprpp/fork.h"
+#include "src/core/lib/gprpp/mutex_lock.h"
 #include "src/core/lib/http/parser.h"
 #include "src/core/lib/iomgr/call_combiner.h"
 #include "src/core/lib/iomgr/combiner.h"
@@ -123,7 +124,7 @@ void grpc_init(void) {
   int i;
   gpr_once_init(&g_basic_init, do_basic_init);
 
-  gpr_mu_lock(&g_init_mu);
+  grpc_core::MutexLock lock(&g_init_mu);
   if (++g_initializations == 1) {
     if (g_shutting_down) {
       g_shutting_down = false;
@@ -159,7 +160,6 @@ void grpc_init(void) {
     grpc_channel_init_finalize();
     grpc_iomgr_start();
   }
-  gpr_mu_unlock(&g_init_mu);
 
   GRPC_API_TRACE("grpc_init(void)", 0, ());
 }
@@ -196,20 +196,18 @@ void grpc_shutdown_internal_locked(void) {
 
 void grpc_shutdown_internal(void* ignored) {
   GRPC_API_TRACE("grpc_shutdown_internal", 0, ());
-  gpr_mu_lock(&g_init_mu);
+  grpc_core::MutexLock lock(&g_init_mu);
   // We have released lock from the shutdown thread and it is possible that
   // another grpc_init has been called, and do nothing if that is the case.
   if (--g_initializations != 0) {
-    gpr_mu_unlock(&g_init_mu);
     return;
   }
   grpc_shutdown_internal_locked();
-  gpr_mu_unlock(&g_init_mu);
 }
 
 void grpc_shutdown(void) {
   GRPC_API_TRACE("grpc_shutdown(void)", 0, ());
-  gpr_mu_lock(&g_init_mu);
+  grpc_core::MutexLock lock(&g_init_mu);
   if (--g_initializations == 0) {
     g_initializations++;
     g_shutting_down = true;
@@ -217,37 +215,33 @@ void grpc_shutdown(void) {
     // currently in an executor thread.
     grpc_core::Thread cleanup_thread(
         "grpc_shutdown", grpc_shutdown_internal, nullptr, nullptr,
-        grpc_core::Thread::Options().set_joinable(false).set_tracked(false));
+        grpc_core::Thread::Options().set_joinable(false));
     cleanup_thread.Start();
   }
-  gpr_mu_unlock(&g_init_mu);
 }
 
 void grpc_shutdown_blocking(void) {
   GRPC_API_TRACE("grpc_shutdown_blocking(void)", 0, ());
-  gpr_mu_lock(&g_init_mu);
+  grpc_core::MutexLock lock(&g_init_mu);
   if (--g_initializations == 0) {
     g_shutting_down = true;
     grpc_shutdown_internal_locked();
   }
-  gpr_mu_unlock(&g_init_mu);
 }
 
 int grpc_is_initialized(void) {
   int r;
   gpr_once_init(&g_basic_init, do_basic_init);
-  gpr_mu_lock(&g_init_mu);
+  grpc_core::MutexLock lock(&g_init_mu);
   r = g_initializations > 0;
-  gpr_mu_unlock(&g_init_mu);
   return r;
 }
 
 void grpc_maybe_wait_for_async_shutdown(void) {
   gpr_once_init(&g_basic_init, do_basic_init);
-  gpr_mu_lock(&g_init_mu);
+  grpc_core::MutexLock lock(&g_init_mu);
   while (g_shutting_down) {
     gpr_cv_wait(g_shutting_down_cv, &g_init_mu,
                 gpr_inf_future(GPR_CLOCK_REALTIME));
   }
-  gpr_mu_unlock(&g_init_mu);
 }

test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc

@@ -18,6 +18,7 @@
 
 #include <cstring>
 
+#include <grpc/grpc.h>
 #include <grpc/support/log.h>
 
 #include "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h"
@@ -27,7 +28,6 @@
 #include "src/core/lib/gprpp/memory.h"
 #include "src/core/lib/iomgr/combiner.h"
 #include "src/core/lib/iomgr/sockaddr_utils.h"
-#include "src/core/lib/surface/init.h"
 #include "test/core/util/test_config.h"
 
 constexpr int kMinResolutionPeriodMs = 1000;
@@ -282,8 +282,7 @@ int main(int argc, char** argv) {
     grpc_core::ExecCtx exec_ctx;
     GRPC_COMBINER_UNREF(g_combiner, "test");
   }
-  grpc_shutdown();
+  grpc_shutdown_blocking();
-  grpc_maybe_wait_for_async_shutdown();
   GPR_ASSERT(g_all_callbacks_invoked);
   return 0;
 }

test/core/end2end/fuzzers/api_fuzzer.cc

@@ -35,7 +35,6 @@
 #include "src/core/lib/iomgr/timer.h"
 #include "src/core/lib/iomgr/timer_manager.h"
 #include "src/core/lib/slice/slice_internal.h"
-#include "src/core/lib/surface/init.h"
 #include "src/core/lib/surface/server.h"
 #include "src/core/lib/transport/metadata.h"
 #include "test/core/end2end/data/ssl_test_data.h"
@@ -1201,7 +1200,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
 
   grpc_resource_quota_unref(g_resource_quota);
 
-  grpc_shutdown();
+  grpc_shutdown_blocking();
-  grpc_maybe_wait_for_async_shutdown();
   return 0;
 }

test/core/end2end/fuzzers/client_fuzzer.cc

@@ -158,6 +158,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
       grpc_byte_buffer_destroy(response_payload_recv);
     }
   }
-  grpc_shutdown();
+  grpc_shutdown_blocking();
   return 0;
 }

test/core/handshake/readahead_handshaker_server_ssl.cc

@@ -37,7 +37,6 @@
 #include "src/core/lib/channel/handshaker_factory.h"
 #include "src/core/lib/channel/handshaker_registry.h"
 #include "src/core/lib/security/transport/security_handshaker.h"
-#include "src/core/lib/surface/init.h"
 
 #include "test/core/handshake/server_ssl_common.h"
 
@@ -84,7 +83,6 @@ int main(int argc, char* argv[]) {
       UniquePtr<HandshakerFactory>(New<ReadAheadHandshakerFactory>()));
   const char* full_alpn_list[] = {"grpc-exp", "h2"};
   GPR_ASSERT(server_ssl_test(full_alpn_list, 2, "grpc-exp"));
-  grpc_shutdown();
+  grpc_shutdown_blocking();
-  grpc_maybe_wait_for_async_shutdown();
   return 0;
 }

test/core/json/fuzzer.cc

@@ -31,8 +31,7 @@ bool leak_check = true;
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
   char* s;
-  struct grpc_memory_counters counters;
+  grpc_core::testing::LeakDetector leak_detector(leak_check);
-  grpc_memory_counters_init();
   s = static_cast<char*>(gpr_malloc(size));
   memcpy(s, data, size);
   grpc_json* x;
@@ -40,8 +39,5 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     grpc_json_destroy(x);
   }
   gpr_free(s);
-  counters = grpc_memory_counters_snapshot();
-  grpc_memory_counters_destroy();
-  GPR_ASSERT(counters.total_size_relative == 0);
   return 0;
 }

test/core/security/ssl_server_fuzzer.cc

@@ -52,9 +52,8 @@ static void on_handshake_done(void* arg, grpc_error* error) {
 }
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
-  struct grpc_memory_counters counters;
   if (squelch) gpr_set_log_function(dont_log);
-  if (leak_check) grpc_memory_counters_init();
+  grpc_core::testing::LeakDetector leak_detector(leak_check);
   grpc_init();
   {
     grpc_core::ExecCtx exec_ctx;
@@ -118,11 +117,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     grpc_core::ExecCtx::Get()->Flush();
   }
 
-  grpc_shutdown();
+  grpc_shutdown_blocking();
-  if (leak_check) {
-    counters = grpc_memory_counters_snapshot();
-    grpc_memory_counters_destroy();
-    GPR_ASSERT(counters.total_size_relative == 0);
-  }
   return 0;
 }