Skip to content
This repository has been archived by the owner on Jun 14, 2023. It is now read-only.

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

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

Just-maple
Copy link
Contributor

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
Copy link

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.

@wu-sheng wu-sheng requested a review from arugal December 7, 2020 07:19
Copy link
Member

@hanahmily hanahmily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hanahmily hanahmily merged commit b266e96 into SkyAPM:master Dec 7, 2020
@arugal arugal linked an issue Jan 13, 2021 that may be closed by this pull request
@arugal arugal mentioned this pull request Jan 13, 2021
@wangrzneu wangrzneu mentioned this pull request Feb 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

客户端协程泄漏
3 participants