From ef5073fb1650d7b72534b0bc529f26ff5f0afded Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 9 Apr 2024 15:05:51 -0700 Subject: [PATCH] [Fix Python Deadlock] Guard grpc_google_default_credentials_create with nogil (#36266) This fix is similar to https://github.com/grpc/grpc/pull/34712 but for `grpc_google_default_credentials_create` rather than `grpc_ssl_credentials_create` Fixes https://github.com/grpc/grpc/issues/36265 Fixes https://github.com/googleapis/python-bigtable/issues/949 Closes #36266 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36266 from parthea:repro-issue-34672 d736f6f080c27a0eddc71b97d2d9b26f81b30e29 PiperOrigin-RevId: 623291826 --- .../grpc/_cython/_cygrpc/credentials.pyx.pxi | 5 +- .../grpcio_tests/tests/unit/_api_test.py | 53 ++++++++++++++++--- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi index 600c0f304e0..181704cb85a 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi @@ -437,8 +437,9 @@ cdef class ComputeEngineChannelCredentials(ChannelCredentials): raise ValueError("Call credentials may not be NULL.") cdef grpc_channel_credentials *c(self) except *: - self._c_creds = grpc_google_default_credentials_create(self._call_creds) - return self._c_creds + with nogil: + self._c_creds = grpc_google_default_credentials_create(self._call_creds) + return self._c_creds def channel_credentials_compute_engine(call_creds): diff --git a/src/python/grpcio_tests/tests/unit/_api_test.py b/src/python/grpcio_tests/tests/unit/_api_test.py index 1824abf08e3..e9e904ff4a0 100644 --- a/src/python/grpcio_tests/tests/unit/_api_test.py +++ b/src/python/grpcio_tests/tests/unit/_api_test.py @@ -112,20 +112,61 @@ class ChannelConnectivityTest(unittest.TestCase): class ChannelTest(unittest.TestCase): - def test_secure_channel(self): - channel_credentials = grpc.ssl_channel_credentials() - channel = grpc.secure_channel("google.com:443", channel_credentials) + def compute_engine_channel_credentials(self): + class TestCallCredentials(grpc.AuthMetadataPlugin): + def __call__(self, context, callback): + callback((), None) + + test_call_credentials = TestCallCredentials() + call_credentials = grpc.metadata_call_credentials( + test_call_credentials, "test call credentials" + ) + return grpc.compute_engine_channel_credentials(call_credentials) + + def test_ssl_secure_channel(self): + channel = grpc.secure_channel( + "google.com:443", grpc.ssl_channel_credentials() + ) + channel.close() + + def test_compute_engine_secure_channel(self): + channel = grpc.secure_channel( + "google.com:443", self.compute_engine_channel_credentials() + ) channel.close() - def test_multiple_secure_channel(self): + def test_multiple_ssl_secure_channel(self): + _THREAD_COUNT = 10 + wait_group = test_common.WaitGroup(_THREAD_COUNT) + + def create_secure_channel(): + wait_group.done() + wait_group.wait() + channel = grpc.secure_channel( + "google.com:443", grpc.ssl_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() + + def test_multiple_compute_engine_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 = grpc.secure_channel( + "google.com:443", self.compute_engine_channel_credentials() + ) channel.close() threads = []