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: with gopgsqldriver Keep-Alive DoS: *db.Query Rows.Close() is not automatic #2624

Closed
patrickmn opened this issue Dec 27, 2011 · 8 comments
Assignees
Milestone

Comments

@patrickmn
Copy link

@patrickmn patrickmn commented Dec 27, 2011

An sql.Rows generated in a request handler is not closed automatically, and DB
connections stay open until the http.Request is closed. (Until conn.Close() is run?) If
the http connection is kept alive, each subsequent request will create a new database
connection until the database's resources are exhausted.

What steps will reproduce the problem?
1. Use github.com/jbarham/pgsqldriver (not sure if this is the culprit; setFinalizer
seems to have been used correctly)
2. Run many Query(); the resulting Rows won't close by themselves even after having been
traversed

What is the expected output? No error; only one/a few DB conns used.

What do you see instead? Hundreds of DB conns, and finally an error: "conn
error:FATAL:  remaining connection slots are reserved for non-replication superuser
connections" (PostgreSQL global maximum connection limit reached.)

Which compiler are you using (5g, 6g, 8g, gccgo)? 6g

Which operating system are you using? Ubuntu 11.10

Which revision are you using?  weekly.2011-12-22

Please provide any additional information below.

PoC: https://gist.github.com/1524739 (run and go to http://localhost:9000/)
@patrickmn
Copy link
Author

@patrickmn patrickmn commented Dec 27, 2011

Comment 1:

The problem does not affect QueryRow.
@patrickmn
Copy link
Author

@patrickmn patrickmn commented Dec 28, 2011

Comment 2:

The actual Git repository is at https://github.com/jbarham/gopgsqldriver, sorry.
@patrickmn
Copy link
Author

@patrickmn patrickmn commented Dec 28, 2011

Comment 3:

The actual Git repository is at https://github.com/jbarham/gopgsqldriver, although name
of the package is pgsqldriver.
@patrickmn
Copy link
Author

@patrickmn patrickmn commented Dec 28, 2011

Comment 4:

This fixes the problem with no observable side effects:
diff -r 4a8268927758 src/pkg/exp/sql/sql.go
--- a/src/pkg/exp/sql/sql.go    Fri Dec 23 14:28:01 2011 +1100
+++ b/src/pkg/exp/sql/sql.go    Wed Dec 28 11:23:45 2011 +0100
@@ -726,6 +726,9 @@
        rs.lastcols = make([]interface{}, len(rs.rowsi.Columns()))
    }
    rs.lasterr = rs.rowsi.Next(rs.lastcols)
+   if rs.lasterr == io.EOF {
+       rs.Close()
+   }
    return rs.lasterr == nil
 }
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 29, 2011

Comment 5:

Owner changed to @bradfitz.

Status changed to Accepted.

@adg
Copy link
Contributor

@adg adg commented Jan 4, 2012

Comment 6:

Labels changed: added priority-go1, removed priority-triage.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 10, 2012

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 10, 2012

Comment 8:

This issue was closed by revision 4435c8b.

Status changed to Fixed.

@mikioh mikioh changed the title exp/sql + gopgsqldriver Keep-Alive DoS: *db.Query Rows.Close() is not automatic database/sql: with gopgsqldriver Keep-Alive DoS: *db.Query Rows.Close() is not automatic Feb 18, 2015
@rsc rsc added this to the Go1 milestone Apr 10, 2015
@rsc rsc removed the priority-go1 label Apr 10, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.