database/sql: fix driver Conn refcounting with prepared statements
The refcounting of driver Conns was completedly busted and
would leak (be held open forever) with any reasonable
load. This was a significant regression from Go 1.0.
The core of this patch is removing one line:
s.db.addDep(dc, s)
A database conn (dc) is a resource that be re-created any time
(but cached for speed) should not be held open forever with a
dependency refcount just because the Stmt (s) is alive (which
typically last for long periods of time, like forever).
The meat of the patch is new tests. In fixing the real issue,
a lot of tests then failed due to the fakedb_test.go's paranoia
about closing a fakeConn while it has open fakeStmts on it. I
could've ignored that, but that's been a problem in the past for
other bugs.
Instead, I now track per-Conn open statements and close them
when the the conn closes. The proper way to do this would've
been making *driverStmt a finalCloser and using the dep mechanism,
but it was much more invasive. Added a TODO instead.
I'd like to give a way for drivers to opt-out of caring about
driver.Stmt closes before a driver.Conn close, but that's a TODO
for the future, and that TODO is added in this CL.
I know this is very late for Go 1.1, but database/sql is
currently nearly useless without this.
I'd like to believe all these database/sql bugs in the past
release cycle are the result of increased usage, number of
drivers, and good feedback from increasingly-capable Go
developers, and not the result of me sucking. It's also hard
with all the real drivers being out-of-tree, so I'm having to
add more and more hooks to fakedb_test.go to simulate things
which real drivers end up doing.
Fixes #5323
R=golang-dev, snaury, gwenn.kahz, google, r
CC=golang-dev
https://golang.org/cl/8836045
Anthony Martin [Tue, 23 Apr 2013 23:02:50 +0000 (16:02 -0700)]
cmd/dist: fix line numbers in goc2c generated files
We have to reset the global lineno variable before
processing each file otherwise line numbers will be
offset by the number of lines in the previous file.
The following examples are from the beginning of the
ztime_linux_amd64.c file which is generated from
time.goc in the runtime package.
Rob Pike [Mon, 22 Apr 2013 18:36:17 +0000 (11:36 -0700)]
time: improve the explanation of the working of Format and Parse
Change the term 'standard time', which already means something,
to 'reference time', and add a couple of sentences and clarifications.
Dave Cheney [Thu, 18 Apr 2013 06:42:56 +0000 (16:42 +1000)]
misc/dashboard/builder: always do a native build
https://golang.org/cl/8134043 disabled cgo when cross compiling, this means builders which compile for both amd64 and 386 will be compiling the latter with cgo disabled.
This proposal modifies the builder to mirror the dist tool by always doing a native build.
Tested on my darwin/amd64 builder and confirmed the result when building darwin/386 is a native 386 build with cgo enabled.
mime/multipart: don't strip leading space/tab in quoted-printable decoding
Late bug fix, but this is arguably a regression from Go 1.0,
since we added this transparent decoding since then. Without
this fix, Go 1.0 users could decode this correctly, but Go 1.1
users would not be able to.
The newly added test is from the RFC itself.
The updated tests had the wrong "want" values before. They
were there to test \r\n vs \n equivalence (which is
unchanged), not leading whitespace.
The skipWhite decoder struct field was added in the battles of
Issue 4771 in revision b3bb265bfecf. It was just a wrong
strategy, from an earlier round of attempts in
https://golang.org/cl/7300092/
Update #4771
Fixes #5295
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/8536045
database/sql: close driver Stmt before releasing Conn
From the issue, which describes it as well as I could:
database/sql assumes that driver.Stmt.Close does not need the
connection.
see database/sql/sql.go:1308:
This puts the Rows' connection back into the idle pool, and
then calls the driver.Stmt.Close method of the Stmt it belongs
to. In the postgresql driver implementation
(https://github.com/lib/pq), Stmt.Close communicates with the
server (on the connection that was just put back into the idle
pool). Most of the time, this causes no problems, but if
another goroutine makes a query at the right (wrong?) time,
chaos results.
In any case, traffic is being sent on "free" connections
shortly after they are freed, leading to race conditions that
kill the driver code.
Fixes #5283
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/8633044
cmd/go: quote command line arguments in debug output
Debug output from go test -x may contain empty arguments.
This CL quotes arguments if needed. E.g. the output of
go test -x is now
.../6g -o ./_go_.6 -p testmain -complete -D "" -I . -I $WORK ./_testmain.go
which is easier to grasp.
R=golang-dev, bradfitz, minux.ma, r
CC=golang-dev
https://golang.org/cl/8633043
cmd/go: quote command line arguments in debug output
Debug output from go test -x may contain empty arguments.
This CL quotes arguments if needed. E.g. the output of
go test -x is now
.../6g -o ./_go_.6 -p testmain -complete -D "" -I . -I $WORK ./_testmain.go
which is easier to grasp.
R=golang-dev, bradfitz, minux.ma, r
CC=golang-dev
https://golang.org/cl/8633043
Ian Lance Taylor [Fri, 12 Apr 2013 12:23:38 +0000 (05:23 -0700)]
runtime: add a hook to disable precise GC
This will let us ask people to rebuild the Go system without
precise GC, and then rebuild and retest their program, to see
if precise GC is causing whatever problem they are having.
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/8700043
Ian Lance Taylor [Fri, 12 Apr 2013 12:20:15 +0000 (05:20 -0700)]
runtime: use UMTX_OP_WAIT_UINT on FreeBSD
UMTX_OP_WAIT expects that the address points to a uintptr, but
the code in lock_futex.c uses a uint32. UMTX_OP_WAIT_UINT is
just like UMTX_OP_WAIT, but the address points to a uint32.
This almost certainly makes no difference on a little-endian
system, but since the kernel supports it we should do the
right thing. And, who knows, maybe it matters.
Rob Pike [Thu, 11 Apr 2013 18:33:25 +0000 (11:33 -0700)]
spec: add another sentence about BOMs
Although one may argue that they should be legal, gc (at least)
disallows byte order marks that are not the first code point
in the file. Added a sentence to the "Implementation restriction"
clause in the "Source code representation" section to document
this better.
Lifting this restriction (again - the rule has changed at least
twice already) would not break any existing programs, should
we later decide yet again to fiddle the rules about these little
fly specks.
Rob Pike [Wed, 10 Apr 2013 21:05:34 +0000 (14:05 -0700)]
fmt: tweak the explanation of Stringer to be more correct and less specific
The String method is called whenever the printing operation wants a string,
not just for %s and %v.
Rob Pike [Wed, 10 Apr 2013 20:05:34 +0000 (13:05 -0700)]
encoding/json: different decision on tags and shadowing
If there are no tags, the rules are the same as before.
If there is a tagged field, choose it if there is exactly one
at the top level of all fields.
More tests. The old tests were clearly inadequate, since
they all pass as is. The new tests only work with the new code.
Rob Pike [Tue, 9 Apr 2013 22:00:21 +0000 (15:00 -0700)]
encoding/json: fix handling of anonymous fields
The old code was incorrect and also broken. It passed the tests by accident.
The new algorithm is:
1) Sort the fields in order of names.
2) For all fields with the same name, sort in increasing depth.
3) Choose the single field with shortest depth.
If any of the fields of a given name has a tag, do the above using
tagged fields of that name only.
Fixes #5245.
Dave Cheney [Tue, 9 Apr 2013 02:48:04 +0000 (12:48 +1000)]
misc/dashboard/builder: add -race builder support
If the build key contains -race, the builder will invoke to the race.{bat,bash} build command. This allows {darwin,linux,windows}-amd64 builders to do race and non race builds in sequence.
Andrew Wilkins [Mon, 8 Apr 2013 23:35:06 +0000 (07:35 +0800)]
cmd/cgo: record CGO_LDFLAGS env var in _cgo_flags
cgo stores cgo LDFLAGS in _cgo_flags and _cgo_defun.c.
The _cgo_defun.c records the flags via
"#pragma cgo_ldflag <flag>", which external linking
relies upon for passing libraries (and search paths)
to the host linker.
The go command will allow LDFLAGS for cgo to be passed
through the environment (CGO_LDFLAGS); cgo ignores
this environment variable, and so its value doesn't
make it into the above mentioned files. This CL changes
cgo to record CGO_LDFLAGS also.
runtime: poor man's heap type info checker
It's not trivial to make a comprehensive check
due to inferior pointers, reflect, gob, etc.
But this is essentially what I've used to debug
the GC issues.
Update #5193.
R=golang-dev, iant, 0xe2.0x9a.0x9b, r
CC=golang-dev
https://golang.org/cl/8455043
Rob Pike [Mon, 8 Apr 2013 06:27:26 +0000 (23:27 -0700)]
cmd/go: fix a misleading sentence
Apologies for not reviewing the previous CL; I was on the road.
The text was misleading; execution is no more separate than with
any other test. Rewritten. Dropped the odd adverb.
Andrew Gerrand [Mon, 8 Apr 2013 05:38:06 +0000 (15:38 +1000)]
archive/zip: handle trailing data after the end of directory header
The spec doesn't explicitly say that trailing data is okay, but a lot
of people do this and most unzippers will handle it just fine. In any
case, this makes the package more useful, and led me to make the
directory parsing code marginally more robust.