diff options
author | 2017-09-09 15:52:11 +0000 | |
---|---|---|
committer | 2017-09-09 21:42:38 +0000 | |
commit | eda5a4db1e88e484d0c45401c345cbc199d633fd (patch) | |
tree | e9ad2c76518d81854aa1585b0704a1306ba3c473 /src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi | |
parent | 8380090f953c193f9c511ee844c3099d3f7e0766 (diff) |
Fix metadata memory leak
The gRPC Core has two styles for passing metadata: as an integer count
along with a grpc_metadata* pointer, which is used for passing metadata
into the core, and as a grpc_metadata_array, which is used for passing
metadata out of the core. The Cython layer of gRPC Python was using a
single data structure wrapping grpc_metadata_array for both purposes,
but this was complex because the core manages the slices contained in
grpc_metadata_array objects (at least those of which it is aware), so
the Cython layer had to keep track of whether or not the core was aware
of the slices it was using (and it was also defective, leaking slices).
This is solved by realigning with the Cython layer’s intended design of
mirroring as closely as possible in Python the gRPC Core API: we use
one structure for passing metadata into the core (what is now called
cygrpc.Metadata) and second, different structure for receiving metadata
out of the core (what was called cygrpc.Metadata but is now
cygrpc.MetadataArray, reflecting that it wraps the core’s
grpc_metadata_array).
All bug fixes should contain added tests preventing regression but this
doesn't because I don't know at this time how to write a does-not-leak
test for Python that fits well into our existing body of tests. Phooey.
Thanks to Dominik Janků (djanku@email.cz) for investigation and an
earlier draft of a solution.
Diffstat (limited to 'src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi')
-rw-r--r-- | src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi | 11 |
1 files changed, 2 insertions, 9 deletions
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 28c30e5d35..237f430799 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi @@ -41,9 +41,8 @@ cdef class CompletionQueue: cdef object user_tag = None cdef Call operation_call = None cdef CallDetails request_call_details = None - cdef Metadata request_metadata = None + cdef object request_metadata = None cdef Operations batch_operations = None - cdef Operation batch_operation = None if event.type == GRPC_QUEUE_TIMEOUT: return Event( event.type, False, None, None, None, None, False, None) @@ -63,14 +62,8 @@ cdef class CompletionQueue: operation_call = tag.operation_call request_call_details = tag.request_call_details if tag.request_metadata is not None: - request_metadata = tag.request_metadata - request_metadata._claim_slice_ownership() + request_metadata = tuple(tag.request_metadata) batch_operations = tag.batch_operations - if tag.batch_operations is not None: - for op in batch_operations.operations: - batch_operation = <Operation>op - if batch_operation._received_metadata is not None: - batch_operation._received_metadata._claim_slice_ownership() if tag.is_new_request: # Stuff in the tag not explicitly handled by us needs to live through # the life of the call |