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

helper/resource: Potential Goroutine Leak with go-plugin #1063

Closed
bflad opened this issue Sep 20, 2022 · 3 comments
Closed

helper/resource: Potential Goroutine Leak with go-plugin #1063

bflad opened this issue Sep 20, 2022 · 3 comments
Assignees
Labels
bug Something isn't working subsystem/tests Issues and feature requests related to the testing framework.
Milestone

Comments

@bflad
Copy link
Contributor

bflad commented Sep 20, 2022

SDK version

v2.23.0

Relevant provider source code

Refer to the terraform-provider-random acceptance testing.

Debug Output

Go timeout panic stacktrace

Expected Behavior

Running go test -count=1 -parallel=1 -timeout=10m -v ./internal/provider would yield a relatively small number of go-plugin-related Goroutines because the testing should be serialized and each test should properly close them.

Actual Behavior

Refer to Gist above, there's a lot of this:

goroutine 10 [select, 9 minutes]:
github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv(0x0?)
	/Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:121 +0x58
github.com/hashicorp/go-plugin.(*GRPCBroker).Run(0x14000046460)
	/Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:411 +0x40
created by github.com/hashicorp/go-plugin.(*GRPCServer).Init
	/Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_server.go:85 +0x424

goroutine 75981 [select]:
github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv(0x1005ab701?)
	/Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:121 +0x58
github.com/hashicorp/go-plugin.(*GRPCBroker).Run(0x140002b2370)
	/Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:411 +0x40
created by github.com/hashicorp/go-plugin.(*GRPCServer).Init
	/Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_server.go:85 +0x424

goroutine 5016 [chan receive, 8 minutes]:
testing.(*testContext).waitParallel(0x14000127e50)
	/opt/homebrew/Cellar/go/1.19.1/libexec/src/testing/testing.go:1557 +0x15c
testing.(*T).Parallel(0x140001d1a00)
	/opt/homebrew/Cellar/go/1.19.1/libexec/src/testing/testing.go:1272 +0x18c
github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource.ParallelTest({0x105717818, 0x140001d1a00}, {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...})
	/Users/bflad/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/v2@v2.23.0/helper/resource/testing.go:652 +0x40
github.com/terraform-providers/terraform-provider-random/internal/provider.TestAccResourcePassword_Keepers_Replace_NullValueToValue(0x140006e8160?)
	/Users/bflad/src/github.com/hashicorp/terraform-provider-random/internal/provider/resource_password_test.go:2147 +0x3c4
testing.tRunner(0x140001d1a00, 0x105701d40)
	/opt/homebrew/Cellar/go/1.19.1/libexec/src/testing/testing.go:1446 +0x10c
created by testing.(*T).Run
	/opt/homebrew/Cellar/go/1.19.1/libexec/src/testing/testing.go:1493 +0x300

goroutine 45613 [select, 5 minutes]:
github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv(0x140002429a0?)
	/Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:121 +0x58
github.com/hashicorp/go-plugin.(*GRPCBroker).Run(0x14000167810)
	/Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:411 +0x40
created by github.com/hashicorp/go-plugin.(*GRPCServer).Init
	/Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_server.go:85 +0x424

Steps to Reproduce

  1. In terraform-provider-random TF_ACC=1 go test -count=1 -parallel=1 -timeout=10m -v ./internal/provider (and have it time out so it raises the stacktrace)
@bflad bflad added bug Something isn't working subsystem/tests Issues and feature requests related to the testing framework. labels Sep 20, 2022
bflad added a commit that referenced this issue Nov 4, 2022
Reference: #1063
Reference: hashicorp/go-plugin#220

This includes the following small bug fixes:

- Clarifies/fixes the trace logging during `terraform show` commands
- Improves performance slightly for `terraform show` raw plan usage (terraform-exec already captures and returns stdout for raw plans)
- Prevents a goroutine leak specific to the detached stop context in `schema.GRPCProvider`

Previously, leak detection on testing would contain an entry per Terraform command such as:

```
        [Goroutine 276 in state select, with github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.mergeStop on top of the stack:
        goroutine 276 [select]:
        github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.mergeStop({0x103a5f298?, 0x14000390c00?}, 0x1400033e5f0, 0x140004bb560)
                /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider.go:46 +0x64
        created by github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).StopContext
                /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider.go:63 +0x178
        ]
```

Leaked goroutines such as:

```
        [Goroutine 53 in state select, with github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv on top of the stack:
        goroutine 53 [select]:
        github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv(0x29?)
                /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:121 +0x58
        github.com/hashicorp/go-plugin.(*GRPCBroker).Run(0x140003ca190)
                /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:411 +0x40
        created by github.com/hashicorp/go-plugin.(*GRPCServer).Init
                /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_server.go:85 +0x424
        ]
```

Will likely require a fix upstream in go-plugin.
bflad added a commit that referenced this issue Nov 7, 2022
Reference: #1063
Reference: hashicorp/go-plugin#220

This includes the following small bug fixes:

- Clarifies/fixes the trace logging during `terraform show` commands
- Improves performance slightly for `terraform show` raw plan usage (terraform-exec already captures and returns stdout for raw plans)
- Prevents a goroutine leak specific to the detached stop context in `schema.GRPCProvider`

Previously, leak detection on testing would contain an entry per Terraform command such as:

```
        [Goroutine 276 in state select, with github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.mergeStop on top of the stack:
        goroutine 276 [select]:
        github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.mergeStop({0x103a5f298?, 0x14000390c00?}, 0x1400033e5f0, 0x140004bb560)
                /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider.go:46 +0x64
        created by github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).StopContext
                /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider.go:63 +0x178
        ]
```

Leaked goroutines such as:

```
        [Goroutine 53 in state select, with github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv on top of the stack:
        goroutine 53 [select]:
        github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv(0x29?)
                /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:121 +0x58
        github.com/hashicorp/go-plugin.(*GRPCBroker).Run(0x140003ca190)
                /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:411 +0x40
        created by github.com/hashicorp/go-plugin.(*GRPCServer).Init
                /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_server.go:85 +0x424
        ]
```

Will likely require a fix upstream in go-plugin.
@bflad
Copy link
Contributor Author

bflad commented Nov 7, 2022

#1091 which will be part of the next release covers the sdk-based provider global StopContext leak. hashicorp/go-plugin#220 (or something similar) would cover the gRPC broker leak.

@bflad bflad self-assigned this Nov 8, 2022
@bflad bflad added this to the v2.24.1 milestone Nov 8, 2022
@bflad
Copy link
Contributor Author

bflad commented Nov 8, 2022

#1095 should cover the gRPC broker leak.

@bflad bflad closed this as completed Nov 8, 2022
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working subsystem/tests Issues and feature requests related to the testing framework.
Projects
None yet
Development

No branches or pull requests

1 participant