From 9218d1066ebf4d728549474e95bf8c0173253e35 Mon Sep 17 00:00:00 2001 From: Yousuk Seung Date: Thu, 19 Sep 2024 13:44:21 -0700 Subject: [PATCH] Revert "Don't grab GIL from C-Core during shutdown (#37762)" This change appears to cause a deadlock. This reverts commit 4b53c2bac3bfd17c6f9ab9f790d63b8c9d5fea67. --- .../grpc/_cython/_cygrpc/credentials.pyx.pxi | 38 +------------------ 1 file changed, 2 insertions(+), 36 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi index 0f9f3f629a0..181704cb85a 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi @@ -12,7 +12,6 @@ # 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) @@ -66,43 +65,11 @@ cdef int _get_metadata(void *state, return 0 # Asynchronous return -# 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(state) - g_shutdown_mu.unlock() +cdef void _destroy(void *state) except * with gil: + cpython.Py_DECREF(state) 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): @@ -116,7 +83,6 @@ cdef class MetadataPluginCallCredentials(CallCredentials): c_metadata_plugin.state = 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.