From 4b53c2bac3bfd17c6f9ab9f790d63b8c9d5fea67 Mon Sep 17 00:00:00 2001
From: Yousuk Seung <ysseung@google.com>
Date: Thu, 19 Sep 2024 10:33:02 -0700
Subject: [PATCH] Don't grab GIL from C-Core during shutdown (#37762)

MetadataPluginCallCredentials passes a function pointer to C-core to be called during destruction. This function grabs GIL which may race between GIL destruction during process shutdown. Since GIL destruction happens after Python's exit handlers, we cana mark that Python is shutting down from an exit handler and don't grab GIL in this function afterwards using a C mutex.

<!--

If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the appropriate
lang label.

-->

Closes #37762

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/37762 from yousukseung:gil-shutdown 945eca9c6ad89031f87ef16bd8be397bac7d94fe
PiperOrigin-RevId: 676471627
---
 .../grpc/_cython/_cygrpc/credentials.pyx.pxi  | 38 ++++++++++++++++++-
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi
index 181704cb85a..0f9f3f629a0 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi
@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import atexit
 
 def _spawn_callback_in_thread(cb_func, args):
   t = ForkManagedThread(target=cb_func, args=args)
@@ -65,11 +66,43 @@ cdef int _get_metadata(void *state,
   return 0  # Asynchronous return
 
 
-cdef void _destroy(void *state) except * with gil:
-  cpython.Py_DECREF(<object>state)
+# Protects access to GIL from _destroy() and to g_shutting_down.
+cdef mutex g_shutdown_mu
+cdef int g_shutting_down = 0
+
+# This is called by C-core when the plugin is destroyed, which may race between
+# GIL destruction during process shutdown. Since GIL destruction happens after
+# Python's exit handlers, we mark that Python is shutting down from an exit
+# handler and don't grab GIL in this function afterwards using a C mutex.
+# Access to g_shutting_down and GIL must be mutex protected here.
+cdef void _destroy(void *state) nogil:
+  global g_shutdown_mu
+  global g_shutting_down
+  g_shutdown_mu.lock()
+  if g_shutting_down == 0:
+    with gil:
+      cpython.Py_DECREF(<object>state)
+  g_shutdown_mu.unlock()
   grpc_shutdown()
 
 
+g_shutdown_handler_registered = False
+
+def _maybe_register_shutdown_handler():
+  global g_shutdown_handler_registered
+  if g_shutdown_handler_registered:
+    return
+  g_shutdown_handler_registered = True
+  atexit.register(_on_shutdown)
+
+def _on_shutdown():
+  global g_shutdown_mu
+  global g_shutting_down
+  g_shutdown_mu.lock()
+  g_shutting_down = 1
+  g_shutdown_mu.unlock()
+
+
 cdef class MetadataPluginCallCredentials(CallCredentials):
 
   def __cinit__(self, metadata_plugin, name):
@@ -83,6 +116,7 @@ cdef class MetadataPluginCallCredentials(CallCredentials):
     c_metadata_plugin.state = <void *>self._metadata_plugin
     c_metadata_plugin.type = self._name
     cpython.Py_INCREF(self._metadata_plugin)
+    _maybe_register_shutdown_handler()
     fork_handlers_and_grpc_init()
     # TODO(yihuazhang): Expose min_security_level via the Python API so that
     # applications can decide what minimum security level their plugins require.