Closed
Description
by johnkgallagher:
What steps will reproduce the problem? This test function demonstrates the problem (note that this won't actually run on play.golang.org because of the dependency on go-sqlite3): http://play.golang.org/p/mKL22tAoxR The code is wrong, but the errors it produces are very confusing and don't at all point to the actual problem. On line 19, a transaction's prepared statement's Close() is deferred. However, the transaction is committed before returning from the function, which means the Close() happens after the Commit. Depending on the driver, this can cause: 1. Silent lockup (observed with both go-sqlite3 and github.com/bmizerany/pq) 2. Very odd driver messages (pq reported nonsensical errors, like "expected 4 arguments, got 1" on a query that had 1 argument) 3. Memory corruption panic (observed with go-sqlite3 (cgo)) What is the expected output? Nothing (success) What do you see instead? Sometimes success, more often either a lockup or "panic: runtime error: invalid memory address or nil pointer dereference". Commenting out line 19 and uncommenting line 29 fixes the problem. Which compiler are you using (5g, 6g, 8g, gccgo)? 6g Which operating system are you using? Mac OS X, Linux Which version are you using? (run 'go version') 1.0.3 Please provide any additional information below. I'm not sure this is necessarily a bug in database/sql, since the documentation does say that a prepared statement can't be used after a transaction is committed/rolled back. However, it would be nicer if it coped with this more gracefully, as it seemed "natural" (although clearly wrong) to defer the Stmt.Close(). Any of the following would have been preferable (to me, at least) than the debugging I did: 1. Transaction closes its open statements inside Commit/Rollback 2. Stmt.Close() panics if it's called outside the transaction 3. Stmt.Close() does nothing if it's called outside the transaction
Metadata
Metadata
Assignees
Type
Projects
Relationships
Development
No branches or pull requests
Activity
bradfitz commentedon Nov 29, 2012
Comment 1:
Owner changed to @bradfitz.
Status changed to Accepted.
rsc commentedon Dec 10, 2012
Comment 2:
Labels changed: added size-m.
rsc commentedon Dec 10, 2012
Comment 3:
Labels changed: added suggested.
rsc commentedon Dec 30, 2012
Comment 4:
Labels changed: added priority-later, removed priority-triage.
gwenn commentedon Jan 12, 2013
Comment 5:
gwenn commentedon Jan 13, 2013
Comment 6:
rsc commentedon Mar 12, 2013
Comment 7:
Labels changed: added go1.1maybe, removed go1.1.
robpike commentedon May 18, 2013
Comment 8:
Labels changed: added go1.2maybe, removed go1.1maybe.
rsc commentedon Sep 9, 2013
Comment 9:
Labels changed: added go1.3maybe, removed go1.2maybe.
rsc commentedon Dec 4, 2013
Comment 10:
Labels changed: added release-none, removed go1.3maybe.
rsc commentedon Dec 4, 2013
Comment 11:
Labels changed: added repo-main.
gopherbot commentedon Nov 2, 2014
Comment 12 by marko@joh.to:
6 remaining items
s7v7nislands commentedon Nov 2, 2016
https://codereview.appspot.com/131650043
should be closed?
bradfitz commentedon Nov 8, 2016
/cc @kardianos
kardianos commentedon Nov 8, 2016
I'll look into it.
kardianos commentedon Nov 16, 2016
Yes, this could be closed.