grpc-dotnet icon indicating copy to clipboard operation
grpc-dotnet copied to clipboard

Bidi/server-streaming calls cause a leak if the response isn't read or disposed

Open jskeet opened this issue 1 year ago • 9 comments

(I haven't tested with a server-only streaming call, but I'd expect it to have the same behavior.)

This occurs in both Grpc.Core and Grpc.Net.Client as far as I can see. Tested with Grpc.Net.Client 2.51.0; I don't have the Grpc.Core version handy, but I can sort that out another time if it's wanted.

A repro is currently available in the "Issue10034" directory of my https://github.com/jskeet/google-cloud-dotnet/tree/issue10034-repro branch - but it's a large repo to pull for just this, so I'd recommend browsing.

The use of the BigQuery API is purely as a simple way of investigating our GAPIC library wrapper behavior as well - the sample doesn't talk to an actual BigQuery API. (You'll need credentials of some form to repro the GAPIC code; let me know if you want the code that just uses a bogus access token for that instead. This issue is focused on the "raw" gRPC layer though.)

Basically, code like this is broken:

var channel = GrpcChannel.ForAddress("https://localhost:5001");
var client = new BigQueryWrite.BigQueryWriteClient(channel);
var request = new AppendRowsRequest { WriteStream = "abc" };
var stream = client.AppendRows();
await stream.RequestStream.WriteAsync(request);
await stream.RequestStream.CompleteAsync();
// Assume desired behavior is fire-and-forget - we don't care about the responses

The code above, as written, will leak memory (permanently as far as I can see).

It can be fixed by either disposing of stream, or by reading all the responses from stream.ResponseStream. I'm perfectly fine with documenting that it's best practice to do both of those - but I don't think we should actually leak memory if the user fails to do so.

jskeet avatar Apr 04 '23 15:04 jskeet