Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detected goroutine leak when updating gRPC dep #6413

Closed
codyoss opened this issue Jun 26, 2023 · 5 comments
Closed

Detected goroutine leak when updating gRPC dep #6413

codyoss opened this issue Jun 26, 2023 · 5 comments
Assignees

Comments

@codyoss
Copy link

codyoss commented Jun 26, 2023

I see a leak in an upgrade deps PR in our project: googleapis/gapic-generator-go#1362

--- FAIL: TestComplianceSuite (5.07s)
    leakcheck.go:111: Leaked goroutine: goroutine 4 [select]:
        google.golang.org/grpc/internal/grpcsync.(*CallbackSerializer).run(0xc000080480, {0xdc0458, 0xc000037540})
        	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.56.1/internal/grpcsync/callback_serializer.go:83 +0x12a
        created by google.golang.org/grpc/internal/grpcsync.NewCallbackSerializer
        	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.56.1/internal/grpcsync/callback_serializer.go:55 +0x138
    leakcheck.go:111: Leaked goroutine: goroutine 5 [select]:
        google.golang.org/grpc/internal/grpcsync.(*CallbackSerializer).run(0xc0000804e0, {0xdc0458, 0xc000037580})
        	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.56.1/internal/grpcsync/callback_serializer.go:83 +0x12a
        created by google.golang.org/grpc/internal/grpcsync.NewCallbackSerializer
        	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.56.1/internal/grpcsync/callback_serializer.go:55 +0x138

Originally posted by @codyoss in #6294 (comment)

@easwars
Copy link
Contributor

easwars commented Jun 27, 2023

Could you please give us more information about what you are trying to do and how to reproduce this leak.

@codyoss
Copy link
Author

codyoss commented Jun 27, 2023

Here is an example of one of the tests that is triggering the leak check.

https://github.com/googleapis/gapic-generator-go/blob/7a05e8d14af242363aa52c08db39a21e513ce227/showcase/echo_test.go#L52-L70

It appears like just about all of the tests in this file are failing though with this dependency update. If you click through to the linked issue I believe our GH action logs are public.

@easwars
Copy link
Contributor

easwars commented Jun 27, 2023

I believe the same logic reason described here applies to this goroutine.

The leaked goroutine that you are seeing is from a component called CallbackSerializer which is used internally in gRPC-Go in multiple places to execute callbacks in a serialized fashion. For example, the name resolver and load balancer wrappers in grpc use this component. This serializer internally uses a goroutine where it reads incoming callbacks and executes them in a serialized fashion.

If the grpc.ClientConn is not closed after each test, the goroutines spawned by the callback serializer will leak, and this is WAI. You might want to think about creating a grpc.ClientConn on a per-test basis.

@easwars easwars closed this as completed Jun 27, 2023
@codyoss
Copy link
Author

codyoss commented Jun 27, 2023

Sounds good, will add a skip for this. Just wanted to verify this was WAI since this was a behavior change between versions.

@easwars
Copy link
Contributor

easwars commented Jun 27, 2023

Yes, we added the callback serializer to the name resolver and load balancer wrappers within gRPC recently.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants