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

callctx: race condition #325

Closed
jjkoh95 opened this issue Feb 22, 2024 · 3 comments · Fixed by #327
Closed

callctx: race condition #325

jjkoh95 opened this issue Feb 22, 2024 · 3 comments · Fixed by #327
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@jjkoh95
Copy link

jjkoh95 commented Feb 22, 2024

callctx.SetHeaders is not concurrent safe.
I'm not sure if this is by design or potentially a bug.
It should be explicit in the comment section if it is designed to be non-concurrent safe.

Environment details

  • Programming language: Go
  • OS: Debian GNU/Linux 12 (bookworm)
  • Language runtime version: go version go1.22.0 linux/amd64
  • Package version: github.com/googleapis/gax-go/v2 v2.12.0

Steps to reproduce

// go run --race main.go
func main() {
	// first call to search places
	cctx := callctx.SetHeaders(context.Background(), "x-goog-fieldmask", "*")
	// for each place, get place details...
	for i := 0; i < 3; i++ {
		go func(ctx context.Context) {
			ctx = callctx.SetHeaders(ctx, "x-goog-fieldmask", "*")
			m := callctx.HeadersFromContext(ctx)
			fmt.Println(m)
		}(cctx)
	}

	fmt.Scanln()
}

For more information, I'm using this to search places in Google Maps, then foreach place, I do parallel calls to retrieve place details where I naively reused the same context and hit this error.

@jjkoh95 jjkoh95 added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Feb 22, 2024
@noahdietz
Copy link
Contributor

Hey @jjkoh95 thanks for this, especially the repro! We will take a look and get back to you, apologies for the friction

@noahdietz noahdietz added type: question Request for information or clarification. Not an issue. status: investigating The issue is under investigation, which is determined to be non-trivial. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. type: question Request for information or clarification. Not an issue. status: investigating The issue is under investigation, which is determined to be non-trivial. labels Feb 23, 2024
@noahdietz
Copy link
Contributor

We found the issue and are fixing it! We definitely want this to be concurrent safe

@noahdietz
Copy link
Contributor

Fix is in, release will be cut on Monday and I will close this. Thanks for the report, it's really appreciated!

@noahdietz noahdietz linked a pull request Feb 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants