fix(spanner): fix createMultiplexedSession goroutine leak by egonelbre · Pull Request #13396 · googleapis/google-cloud-go

egonelbre · 2025-11-21T17:43:49Z

No description provided.

gemini-code-assist · 2025-11-21T17:44:03Z

Summary of Changes

Hello @egonelbre, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a goroutine leak within the Spanner client's session management. By explicitly closing the multiplexedSessionReq channel when the session pool is shut down, it prevents goroutines from remaining active indefinitely, thereby improving resource cleanup and overall application stability.

Highlights

Goroutine Leak Fix: Addressed a goroutine leak in the createMultiplexedSession mechanism by ensuring the associated channel is properly closed during session pool shutdown.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request aims to fix a goroutine leak by closing the multiplexedSessionReq channel when the session pool is closed. While this correctly terminates the consumer goroutine, the placement of the close() call introduces a critical race condition. Closing the channel after releasing the mutex can lead to a panic if another goroutine attempts to send a request concurrently with the pool closure. I've recommended moving the close() call to be within the mutex-protected block to ensure atomicity and prevent this potential panic.

spanner/session.go

rahul2393 · 2025-11-24T17:10:22Z

@olavloite / @sakthivelmanii need help in one more approval to merge this one, please help

olavloite · 2025-11-25T07:07:38Z

spanner/session.go

 	}
 	p.mu.Unlock()
 	p.hc.close()
+	close(p.multiplexedSessionReq)


This change would get lost if we remove the entire sessionPool implementation (which is the intention). So it might be better to put this line of code in the Client#Close() function instead to make sure it is not unintentionally deleted. AFAICT, there are no tests that verify that this is actually closed, meaning that it would also not be caught by any tests.

I think the property that should be enforced is that all started goroutines are stopped. If the channels is left open and no one is using it, then it probably doesn't matter.

Using synctest seems a good fit for this. However, it does require Go 1.25 or 1.24 with GOEXPERIMENT=synctest. I wouldn't want to bump minimum language version requirement just for that... however the test could be put behind build tag.

I created a draft for that test, however it fails at the moment #13406. Need to diagnose the issue, might require some additional fixes.

SG. I didn't really realize that the entire multiplexed session implementation lives in the session pool, so my comment that this call should be moved to outside the pool does not really make any sense.

I managed to fix #13406 and it succeeds when this PR is merged. However, the mock server still leaks, which I excluded from the check.

…#13396)

PR created by the Librarian CLI to initialize a release. Merging this PR will auto trigger a release. Librarian Version: v1.0.0 Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/librarian-go@sha256:718167d5c23ed389b41f617b3a00ac839bdd938a6bd2d48ae0c2f1fa51ab1c3d <details><summary>spanner: 1.87.0</summary> ## [1.87.0](spanner/v1.86.1...spanner/v1.87.0) (2025-12-10) ### Features * Add Send and Ack mutations for Queues (PiperOrigin-RevId: 832425466) ([185951b](185951b3)) * Exposing AutoscalingConfig in InstancePartition (PiperOrigin-RevId: 825184314) ([185951b](185951b3)) * Add Spanner location API (PiperOrigin-RevId: 833474957) ([185951b](185951b3)) * Add QueryAdvisorResult for query plan (PiperOrigin-RevId: 832425466) ([185951b](185951b3)) * improve the SQL formatting when printing out SQL (#13267) ([af0806f](af0806f4)) * Add grpc.xds.resource_type label to xDS client metrics (#13358) ([b9196cf](b9196cf6)) * support subquery in View Join (#13266) ([d19f797](d19f797b)) ### Bug Fixes * add env var to allow disabling directpath bound token (#13265) ([029bc79](029bc795)) * fix createMultiplexedSession goroutine leak (#13396) ([1805e89](1805e895)) * decoding spanner rows using SelectAll should map values in correct annotations (#13301) ([315f65b](315f65b5)) * error instead of panic for iterator after tx end (#13366) ([a27c19a](a27c19ae)) * transaction_tag should be set on BeginTransactionRequest (#13463) ([a429aea](a429aea4)) * Configure keepAlive time for gRPC TCP connections (#13216) ([ca8f64e](ca8f64e0)) * avoid double decrement in session counting (#13395) ([e036421](e0364214)) ### Documentation * minor update for Spanner Location API (PiperOrigin-RevId: 834841888) ([185951b](185951b3)) * Update description for the BatchCreateSessionsRequest and Session (PiperOrigin-RevId: 832425466) ([185951b](185951b3)) * Update description for the IsolationLevel (PiperOrigin-RevId: 832425466) ([185951b](185951b3)) </details>

…3464) PR created by the Librarian CLI to initialize a release. Merging this PR will auto trigger a release. Librarian Version: v1.0.0 Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/librarian-go@sha256:718167d5c23ed389b41f617b3a00ac839bdd938a6bd2d48ae0c2f1fa51ab1c3d <details><summary>spanner: 1.87.0</summary> ## [1.87.0](googleapis/google-cloud-go@spanner/v1.86.1...spanner/v1.87.0) (2025-12-10) ### Features * Add Send and Ack mutations for Queues (PiperOrigin-RevId: 832425466) ([185951b](googleapis@185951b3)) * Exposing AutoscalingConfig in InstancePartition (PiperOrigin-RevId: 825184314) ([185951b](googleapis@185951b3)) * Add Spanner location API (PiperOrigin-RevId: 833474957) ([185951b](googleapis@185951b3)) * Add QueryAdvisorResult for query plan (PiperOrigin-RevId: 832425466) ([185951b](googleapis@185951b3)) * improve the SQL formatting when printing out SQL (googleapis#13267) ([af0806f](googleapis@af0806f4)) * Add grpc.xds.resource_type label to xDS client metrics (googleapis#13358) ([b9196cf](googleapis@b9196cf6)) * support subquery in View Join (googleapis#13266) ([d19f797](googleapis@d19f797b)) ### Bug Fixes * add env var to allow disabling directpath bound token (googleapis#13265) ([029bc79](googleapis@029bc795)) * fix createMultiplexedSession goroutine leak (googleapis#13396) ([1805e89](googleapis@1805e895)) * decoding spanner rows using SelectAll should map values in correct annotations (googleapis#13301) ([315f65b](googleapis@315f65b5)) * error instead of panic for iterator after tx end (googleapis#13366) ([a27c19a](googleapis@a27c19ae)) * transaction_tag should be set on BeginTransactionRequest (googleapis#13463) ([a429aea](googleapis@a429aea4)) * Configure keepAlive time for gRPC TCP connections (googleapis#13216) ([ca8f64e](googleapis@ca8f64e0)) * avoid double decrement in session counting (googleapis#13395) ([e036421](googleapis@e0364214)) ### Documentation * minor update for Spanner Location API (PiperOrigin-RevId: 834841888) ([185951b](googleapis@185951b3)) * Update description for the BatchCreateSessionsRequest and Session (PiperOrigin-RevId: 832425466) ([185951b](googleapis@185951b3)) * Update description for the IsolationLevel (PiperOrigin-RevId: 832425466) ([185951b](googleapis@185951b3)) </details>

fix(spanner): fix createMultiplexedSession goroutine leak

82d2497

egonelbre requested review from a team November 21, 2025 17:43

egonelbre requested a review from a team as a code owner November 21, 2025 17:43

product-auto-label bot added the api: spanner Issues related to the Spanner API. label Nov 21, 2025

egonelbre mentioned this pull request Nov 21, 2025

fix(spanner): avoid double decrement in session counting #13395

Merged

gemini-code-assist bot reviewed Nov 21, 2025

View reviewed changes

spanner/session.go Show resolved Hide resolved

rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 21, 2025

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 21, 2025

rahul2393 approved these changes Nov 24, 2025

View reviewed changes

olavloite reviewed Nov 25, 2025

View reviewed changes

olavloite approved these changes Nov 25, 2025

View reviewed changes

rahul2393 merged commit 1805e89 into googleapis:main Nov 25, 2025
9 of 10 checks passed

egonelbre deleted the fix-multiplexed-session-leak branch November 25, 2025 15:50

krishnamd-jkp pushed a commit to krishnamd-jkp/google-cloud-go that referenced this pull request Dec 8, 2025

fix(spanner): fix createMultiplexedSession goroutine leak (googleapis…

caa2243

…#13396)

rahul2393 mentioned this pull request Dec 10, 2025

chore: librarian release pull request: 20251210T223550Z #13464

Merged

MASA-JAPAN mentioned this pull request Jan 2, 2026

Add Test Coverage for Multiplexed Session Goroutine Leak #13529

Open

6 tasks

anowardear062-svg approved these changes Jan 2, 2026

View reviewed changes

celia-vytrac mentioned this pull request Feb 10, 2026

Secret Manager validation fails when using versions/latest transparency-dev/tesseract#689

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(spanner): fix createMultiplexedSession goroutine leak#13396

fix(spanner): fix createMultiplexedSession goroutine leak#13396
rahul2393 merged 1 commit intogoogleapis:mainfrom
egonelbre:fix-multiplexed-session-leak

egonelbre commented Nov 21, 2025

Uh oh!

gemini-code-assist bot commented Nov 21, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

rahul2393 commented Nov 24, 2025

Uh oh!

olavloite Nov 25, 2025

Uh oh!

egonelbre Nov 25, 2025

Uh oh!

egonelbre Nov 25, 2025

Uh oh!

olavloite Nov 25, 2025

Uh oh!

egonelbre Nov 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

egonelbre commented Nov 21, 2025

Uh oh!

gemini-code-assist bot commented Nov 21, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

rahul2393 commented Nov 24, 2025

Uh oh!

olavloite Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

egonelbre Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

egonelbre Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

olavloite Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

egonelbre Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants