grpc / grpc-swift

The Swift language implementation of gRPC.
Apache License 2.0
2k stars 413 forks source link

Metadata.add Memory Leak #746

Open ryantstone opened 4 years ago

ryantstone commented 4 years ago

Question Checklist

Question Subject

Leak in Metadata

Question Description

When instantiating Metadata objects I'm getting a large number of memory leaks, initially we were using Metadata.add(key:value:) for passing traits but to verify we created the entire dictionary and passed it to the Metadata initializer. While investigating the leaks I saw the comment:

fileprivate let underlyingArray: UnsafeMutableRawPointer /// Ownership of the fields inside metadata arrays provided by grpc_op_recv_initial_metadata and /// grpc_op_recv_status_on_client is retained by the gRPC library. Similarly, passing metadata to gRPC for sending /// to the client for sending/receiving also transfers ownership. However, before we have passed that metadata to /// gRPC, we are still responsible for releasing its fields. This variable tracks that.

Does this mean that metadata is expected to display a retain cycle in and that it should be disregarded or more likely point to a misuse of the API? Commenting out the metadata completely gets rid of the leaks.

Screen_Shot_2020-03-12_at_1_28_40_PM
MrMage commented 4 years ago

When you instantiate a Metadata object manually, it sets ownsFields to true, causing a call to cgrpc_metadata_array_unref_fields upon deinit. In theory, this should dereference (and thus deallocate) the slices that seem to be leaked here. Upon a cursory look, I can't figure out why this is not the case. But I can't guarantee that we aren't using the CgRPC API wrong, either.

ryantstone commented 4 years ago

@MrMage Is there any more information about our usage I can provide to help? I don't want you chasing a rabbit if it has something to do with our implementation.

MrMage commented 4 years ago

To be honest, I don't really have time to look into this right now, anyway. The easiest way to check whether it is your code could be to just instantiate a Metadata object with one pair, then check if the leak still occurs once that object leaves scope.

ryantstone commented 4 years ago

In that case yes, that is what we are doing and the leak is happening. If there is anything else I can do to help let me know. Thanks.