Hold onto the GIL in __dealloc__ functions
When a child thread triggers __dealloc__ as part of a thread being
joined, the thread is already considered to be "joined", and so
releasing the GIL can allow the main thread to proceed to exit,
which introduces shutdown race conditions/memory leaks.
diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/call.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/call.pyx.pxi
index 6570dcd..ba60986 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/call.pyx.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/call.pyx.pxi
@@ -105,8 +105,7 @@
def __dealloc__(self):
if self.c_call != NULL:
- with nogil:
- grpc_call_destroy(self.c_call)
+ grpc_call_destroy(self.c_call)
# The object *should* always be valid from Python. Used for debugging.
@property
diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi
index 1406696..5416401 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi
@@ -102,5 +102,4 @@
def __dealloc__(self):
if self.c_channel != NULL:
- with nogil:
- grpc_channel_destroy(self.c_channel)
+ grpc_channel_destroy(self.c_channel)
diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi
index 9026651..5955021 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi
@@ -118,18 +118,14 @@
def __dealloc__(self):
cdef gpr_timespec c_deadline
- with nogil:
- c_deadline = gpr_inf_future(GPR_CLOCK_REALTIME)
+ c_deadline = gpr_inf_future(GPR_CLOCK_REALTIME)
if self.c_completion_queue != NULL:
# Ensure shutdown
if not self.is_shutting_down:
- with nogil:
- grpc_completion_queue_shutdown(self.c_completion_queue)
- # Pump the queue
+ grpc_completion_queue_shutdown(self.c_completion_queue)
+ # Pump the queue (All outstanding calls should have been cancelled)
while not self.is_shutdown:
- with nogil:
- event = grpc_completion_queue_next(
- self.c_completion_queue, c_deadline, NULL)
+ event = grpc_completion_queue_next(
+ self.c_completion_queue, c_deadline, NULL)
self._interpret_event(event)
- with nogil:
- grpc_completion_queue_destroy(self.c_completion_queue)
+ grpc_completion_queue_destroy(self.c_completion_queue)
diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi
index b24e692..035ac49 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi
@@ -46,8 +46,7 @@
def __dealloc__(self):
if self.c_credentials != NULL:
- with nogil:
- grpc_channel_credentials_release(self.c_credentials)
+ grpc_channel_credentials_release(self.c_credentials)
cdef class CallCredentials:
@@ -64,8 +63,7 @@
def __dealloc__(self):
if self.c_credentials != NULL:
- with nogil:
- grpc_call_credentials_release(self.c_credentials)
+ grpc_call_credentials_release(self.c_credentials)
cdef class ServerCredentials:
@@ -76,8 +74,7 @@
def __dealloc__(self):
if self.c_credentials != NULL:
- with nogil:
- grpc_server_credentials_release(self.c_credentials)
+ grpc_server_credentials_release(self.c_credentials)
cdef class CredentialsMetadataPlugin:
diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi
index b39b2f0..54b3d00 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi
@@ -287,8 +287,7 @@
def __dealloc__(self):
if self.c_byte_buffer != NULL:
- with nogil:
- grpc_byte_buffer_destroy(self.c_byte_buffer)
+ grpc_byte_buffer_destroy(self.c_byte_buffer)
cdef class SslPemKeyCertPair:
@@ -420,8 +419,7 @@
# this frees the allocated memory for the grpc_metadata_array (although
# it'd be nice if that were documented somewhere...)
# TODO(atash): document this in the C core
- with nogil:
- grpc_metadata_array_destroy(&self.c_metadata_array)
+ grpc_metadata_array_destroy(&self.c_metadata_array)
def __len__(self):
return self.c_metadata_array.count
@@ -530,8 +528,7 @@
# Python. The remaining one(s) are primitive fields filled in by GRPC core.
# This means that we need to clean up after receive_status_on_client.
if self.c_op.type == GRPC_OP_RECV_STATUS_ON_CLIENT:
- with nogil:
- gpr_free(self._received_status_details)
+ gpr_free(self._received_status_details)
def operation_send_initial_metadata(Metadata metadata, int flags):
cdef Operation op = Operation()
diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi
index 3e03b6e..4f2d51b 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi
@@ -171,5 +171,4 @@
# much but repeatedly release the GIL and wait
while not self.is_shutdown:
time.sleep(0)
- with nogil:
- grpc_server_destroy(self.c_server)
+ grpc_server_destroy(self.c_server)