From ccfc5b92e48de560a5cfa6972f5bb71f0904eb2d Mon Sep 17 00:00:00 2001
From: Xuan Wang <xuanwn@google.com>
Date: Wed, 18 Oct 2023 09:40:45 -0700
Subject: [PATCH] [Fix Python Deadlock] Guard grpc_ssl_credentials_create with
 nogil (#34712)

Fix: https://github.com/grpc/grpc/issues/34672
With some recent changes in core, now `grpc_ssl_credentials_create` is
guarded by `gpr_once_init`. In our current implementation, The thread
got `gpr_once_init` lock might require GIL lock during the execution of
`grpc_ssl_credentials_create`, which might cause a deadlock if another
thread is holding GIL lock and waiting for `gpr_once_init` lock.

This change adds `with nogil` to calls to native function
`grpc_ssl_credentials_create` to make sure GIL is released before
calling `grpc_ssl_credentials_create`.
<!--

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.

-->
---
 .../grpc/_cython/_cygrpc/credentials.pyx.pxi  | 10 ++++----
 .../grpcio_tests/tests/unit/_api_test.py      | 23 +++++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi
index 27b56aa378b..74a3f16d72d 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi
@@ -153,8 +153,9 @@ cdef class SSLChannelCredentials(ChannelCredentials):
     else:
       c_pem_root_certificates = self._pem_root_certificates
     if self._private_key is None and self._certificate_chain is None:
-      return grpc_ssl_credentials_create(
-          c_pem_root_certificates, NULL, NULL, NULL)
+      with nogil:
+        return grpc_ssl_credentials_create(
+            c_pem_root_certificates, NULL, NULL, NULL)
     else:
       if self._private_key:
         c_pem_key_certificate_pair.private_key = self._private_key
@@ -164,8 +165,9 @@ cdef class SSLChannelCredentials(ChannelCredentials):
         c_pem_key_certificate_pair.certificate_chain = self._certificate_chain
       else:
         c_pem_key_certificate_pair.certificate_chain = NULL
-      return grpc_ssl_credentials_create(
-          c_pem_root_certificates, &c_pem_key_certificate_pair, NULL, NULL)
+      with nogil:
+        return grpc_ssl_credentials_create(
+            c_pem_root_certificates, &c_pem_key_certificate_pair, NULL, NULL)
 
 
 cdef class CompositeChannelCredentials(ChannelCredentials):
diff --git a/src/python/grpcio_tests/tests/unit/_api_test.py b/src/python/grpcio_tests/tests/unit/_api_test.py
index 10a5fefcf56..1824abf08e3 100644
--- a/src/python/grpcio_tests/tests/unit/_api_test.py
+++ b/src/python/grpcio_tests/tests/unit/_api_test.py
@@ -14,11 +14,13 @@
 """Test of gRPC Python's application-layer API."""
 
 import logging
+import threading
 import unittest
 
 import grpc
 
 from tests.unit import _from_grpc_import_star
+from tests.unit import test_common
 
 
 class AllTest(unittest.TestCase):
@@ -115,6 +117,27 @@ class ChannelTest(unittest.TestCase):
         channel = grpc.secure_channel("google.com:443", channel_credentials)
         channel.close()
 
+    def test_multiple_secure_channel(self):
+        _THREAD_COUNT = 10
+        wait_group = test_common.WaitGroup(_THREAD_COUNT)
+
+        def create_secure_channel():
+            channel_credentials = grpc.ssl_channel_credentials()
+            wait_group.done()
+            wait_group.wait()
+            channel = grpc.secure_channel("google.com:443", channel_credentials)
+            channel.close()
+
+        threads = []
+        for _ in range(_THREAD_COUNT):
+            thread = threading.Thread(target=create_secure_channel)
+            thread.setDaemon(True)
+            thread.start()
+            threads.append(thread)
+
+        for thread in threads:
+            thread.join()
+
 
 if __name__ == "__main__":
     logging.basicConfig()