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

spanner: If panic occurs in ReadWriteTransaction, the session will leak #3460

Closed
8398a7 opened this issue Dec 14, 2020 · 0 comments · Fixed by #3461
Closed

spanner: If panic occurs in ReadWriteTransaction, the session will leak #3460

8398a7 opened this issue Dec 14, 2020 · 0 comments · Fixed by #3461
Assignees
Labels
api: spanner Issues related to the Spanner API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@8398a7
Copy link
Contributor

8398a7 commented Dec 14, 2020

Client

Spanner

Environment

macOS Catalina

Go Environment

$ go version

go version go1.15.6 darwin/amd64

$ go env

Code

e.g.

package main

import (
	"context"
	"fmt"
	"log"

	"cloud.google.com/go/spanner"
)

const db = "projects/your-project/instances/your-instance/databases/your-db"

func main() {
	ctx := context.Background()
	client, _ := spanner.NewClientWithConfig(ctx, db, spanner.ClientConfig{
		SessionPoolConfig: spanner.SessionPoolConfig{MaxOpened: 5},
	})

	for i := 0; i < 5; i++ {
		func() {
			defer func() { recover() }()

			_, err := client.ReadWriteTransaction(ctx, func(ctx context.Context, tx *spanner.ReadWriteTransaction) error {
				panic("test")
				return nil
			})
			if err != nil {
				log.Println(err)
			}
		}()
		fmt.Println(i)
	}

	log.Println("start")
	_, err := client.ReadWriteTransaction(ctx, func(ctx context.Context, tx *spanner.ReadWriteTransaction) error {
		return nil
	})
	log.Println("end")
	if err != nil {
		log.Println(err)
	}
}

Expected behavior

When a panic occurs in ReadWriteTransaction, the session will leak.
I want to solve this problem.

I think that I need to rebuild the client to recover from the leaked session, but this is difficult to take care of.

Actual behavior

This is the output result. The last result is not returned.

0
1
2
3
4
2020/12/14 21:54:28 start

If you set the loop count to 4, it will return.

0
1
2
3
2020/12/14 21:55:46 start
2020/12/14 21:55:46 end

Additional context

I have a proposed fix for the problem and will create a pull request.

go.mod

module spanner-session-leak

go 1.15

require cloud.google.com/go/spanner v1.12.0
@8398a7 8398a7 added the triage me I really want to be triaged. label Dec 14, 2020
8398a7 added a commit to 8398a7/google-cloud-go that referenced this issue Dec 14, 2020
@codyoss codyoss added the api: spanner Issues related to the Spanner API. label Dec 14, 2020
@skuruppu skuruppu added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed triage me I really want to be triaged. labels Dec 15, 2020
8398a7 added a commit to 8398a7/google-cloud-go that referenced this issue Dec 15, 2020
gcf-merge-on-green bot pushed a commit that referenced this issue Dec 16, 2020
Fixed a problem where the session was not reused when a panic occurred.

Fixes #3460
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to 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.

3 participants