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

[Backport 5.2] Fix data access race condition in gRPC retry logic #59488

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

sourcegraph-release-bot
Copy link
Collaborator

		callOpts := reuseOrNewWithCallOptions(intOpts, retryOpts) // callOpts are recycled here if no call opts given

		doTrace := makeTracingCallback(parentCtx, logger, service, method)
		originalCallback := callOpts.onRetryCallback
		callOpts.onRetryCallback = func(ctx context.Context, attempt uint, err error) { // and is overwritten here, potentially by multiple callers at the same time.
			doTrace(attempt, err)

			if originalCallback != nil {
				originalCallback(ctx, attempt, err)
			}
		}

Test plan

go test ./cmd/gitserver/internal/integration_tests -race -count=5 consistently fails before this change on branch es/grpc-cleanup where we switched the integration test suite from HTTP to gRPC (the default option). With this patch applied, it no longer fails.
Backport adb95ea from #59487

@sourcegraph-release-bot sourcegraph-release-bot added cla-signed backports team/source Tickets under the purview of Source - the one Source to graph it all backported-to-5.2 labels Jan 10, 2024
@ggilmore
Copy link
Contributor

cc @sourcegraph/release-guild Please stamp, thanks!

@sourcegraph-bot
Copy link
Contributor

📖 Storybook live preview

@DaedalusG DaedalusG enabled auto-merge (squash) January 10, 2024 20:16
@DaedalusG DaedalusG merged commit 4ad658d into 5.2 Jan 10, 2024
21 of 27 checks passed
@DaedalusG DaedalusG deleted the backport-59487-to-5.2 branch January 10, 2024 20:20
@varungandhi-src varungandhi-src mentioned this pull request Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported-to-5.2 backports cla-signed team/source Tickets under the purview of Source - the one Source to graph it all
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants