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

database/sql: close Rows if Conn is closed #33938

Open
Drahflow opened this issue Aug 29, 2019 · 3 comments

Comments

@Drahflow
Copy link

commented Aug 29, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.9 linux/amd64

Does this issue reproduce with the latest release?

Reproduces on play.golang.org

What did you do?

https://play.golang.org/p/jMMhYBt-K9T

I have also locally tested it with https://godoc.org/github.com/lib/pq, same result.

What did you expect to see?

"Hello, playground"

What did you see instead?

fatal error: all goroutines are asleep - deadlock!

goroutine 1 [semacquire]:
sync.runtime_SemacquireMutex(0x43024c, 0x100)
	/usr/local/go/src/runtime/sema.go:71 +0x40
sync.(*RWMutex).Lock(0x430244, 0x40e058)
	/usr/local/go/src/sync/rwmutex.go:98 +0xa0
database/sql.(*Conn).close(0x430240, 0x0, 0x0, 0x1a2ebc, 0x0, 0x0)
	/usr/local/go/src/database/sql/sql.go:1845 +0x60
database/sql.(*Conn).Close(0x430240, 0x1, 0x430240, 0x0)
	/usr/local/go/src/database/sql/sql.go:1860 +0x40
main.TestDatabase()
	/tmp/sandbox755004132/prog.go:60 +0x9bb
main.main()
	/tmp/sandbox755004132/prog.go:63 +0x20

goroutine 5 [select]:
database/sql.(*DB).connectionOpener(0x44c120, 0x1be8a0, 0x43e320, 0x161)
	/usr/local/go/src/database/sql/sql.go:1000 +0xe0
created by database/sql.OpenDB
	/usr/local/go/src/database/sql/sql.go:670 +0x1a0

goroutine 6 [select]:
database/sql.(*DB).connectionResetter(0x44c120, 0x1be8a0, 0x43e320, 0x161)
	/usr/local/go/src/database/sql/sql.go:1013 +0x100
created by database/sql.OpenDB
	/usr/local/go/src/database/sql/sql.go:671 +0x1e0

Program exited: status 2.

@julieqiu julieqiu changed the title Deadlock when not calling rows.Close() correctly. database/sql: deadlock when not calling rows.Close() correctly Aug 29, 2019

@julieqiu

This comment has been minimized.

Copy link

commented Aug 29, 2019

@kardianos

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2019

Cause:

  1. You are pulling out a single connection from the pool.
  2. You are not closing a row, either by finishing reading rows or by calling rows.Close()
  3. The conn is waiting for the row to be done, which it never is.

I'm not sure this is a problem, much less fixable.

maybe Conn.Close could ensure any rows are also closed.

@Drahflow

This comment has been minimized.

Copy link
Author

commented Aug 29, 2019

I'm not sure this is a problem

... until your unit tests mysteriously stop working (and don't report deadlock because of httptest.Server running in parallel).

maybe Conn.Close could ensure any rows are also closed.

Another approach might be to include some stronger wording in the documentation. Instead of
https://golang.org/pkg/database/sql/#Rows.Close

Close closes the Rows, preventing further enumeration. [...]

maybe

Close closes the Rows, preventing further enumeration and allowing the connection to free associated resources. [...]

@kardianos kardianos changed the title database/sql: deadlock when not calling rows.Close() correctly proposal: database/sql: close Rows if Conn is closed Aug 29, 2019

@gopherbot gopherbot added this to the Proposal milestone Aug 29, 2019

@katiehockman katiehockman changed the title proposal: database/sql: close Rows if Conn is closed database/sql: close Rows if Conn is closed Sep 3, 2019

@katiehockman katiehockman modified the milestones: Proposal, Go1.11.14, Go1.14 Sep 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.