aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/python/grpcio
diff options
context:
space:
mode:
authorGravatar Ken Payson <kpayson@google.com>2016-07-11 10:38:39 -0700
committerGravatar Ken Payson <kpayson@google.com>2016-07-11 10:38:39 -0700
commit136ea365e37b611f6b3f7ab94f221737ad3e1cc6 (patch)
tree5a62352facb5a6523f6d03e8aa1080bb038e2107 /src/python/grpcio
parentc1bfe124ab0d151f7619f4fefc08244bd1dd4750 (diff)
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.
Diffstat (limited to 'src/python/grpcio')
-rw-r--r--src/python/grpcio/grpc/_cython/_cygrpc/call.pyx.pxi3
-rw-r--r--src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi3
-rw-r--r--src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi16
-rw-r--r--src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi9
-rw-r--r--src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi9
-rw-r--r--src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi3
6 files changed, 15 insertions, 28 deletions
diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/call.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/call.pyx.pxi
index 6570dcdb85..ba60986143 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 @@ cdef class Call:
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 1406696510..5416401431 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 @@ cdef class Channel:
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 90266516fe..5955021ceb 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 @@ cdef class CompletionQueue:
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 b24e69243e..035ac49a8b 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 @@ cdef class ChannelCredentials:
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 @@ cdef class CallCredentials:
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 @@ cdef class ServerCredentials:
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 b39b2f08de..54b3d00dfc 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 @@ cdef class ByteBuffer:
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 @@ cdef class Metadata:
# 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 @@ cdef class Operation:
# 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 3e03b6efe1..4f2d51b03f 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 @@ cdef class Server:
# 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)