SkyAPM / go2sky

Distributed tracing and monitor SDK in Go for Apache SkyWalking APM
https://skywalking.apache.org/
Apache License 2.0
447 stars 122 forks source link

fix(grpc-reporter): fix close stream to avoid goroutine leaks #80

Closed Just-maple closed 3 years ago

Just-maple commented 3 years ago

Hello,there is an usage mistake on grpc stream control

here is the note from grpc doc

note that when stream is abort,one of those action should be done but now none of them was execute

it lead a result that ,when stream connection recreate(such as using load balance),a goroutine created from grpc.newClientStream leaks

https://godoc.org/google.golang.org/grpc#ClientConn.NewStream

1. Call Close on the ClientConn.
2. Cancel the context provided.
3. Call RecvMsg until a non-nil error is returned. A protobuf-generated
   client-streaming RPC, for instance, might use the helper function
   CloseAndRecv (note that CloseSend does not Recv, therefore is not
   guaranteed to release all resources).
4. Receive a non-nil, non-io.EOF error from Header or SendMsg.
codecov-io commented 3 years ago

Codecov Report

Merging #80 (c78d7ea) into master (38c3b84) will not change coverage. The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #80   +/-   ##
=======================================
  Coverage   67.26%   67.26%           
=======================================
  Files          13       13           
  Lines         559      559           
=======================================
  Hits          376      376           
  Misses        149      149           
  Partials       34       34           
Impacted Files Coverage Δ
reporter/grpc.go 59.41% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 38c3b84...c78d7ea. Read the comment docs.