sql.RawBytes was added the very first Go release, Go 1. Its docs
say:
> RawBytes is a byte slice that holds a reference to memory owned by
> the database itself. After a Scan into a RawBytes, the slice is only
> valid until the next call to Next, Scan, or Close.
That "only valid until the next call" bit was true at the time,
until contexts were added to database/sql in Go 1.8.
In the past ~dozen releases it's been unsafe to use QueryContext with
a context that might become Done to get an *sql.Rows that's scanning
into a RawBytes. The Scan can succeed, but then while the caller's
reading the memory, a database/sql-managed goroutine can see the
context becoming done and call Close on the database/sql/driver and
make the caller's view of the RawBytes memory no longer valid,
introducing races, crashes, or database corruption. See #60304
and #53970 for details.
This change does the minimal surgery on database/sql to make it safe
again: Rows.Scan was already acquiring a mutex to check whether the
rows had been closed, so this change make Rows.Scan notice whether
*RawBytes was used and, if so, doesn't release the mutex on exit
before returning. That mean it's still locked while the user code
operates on the RawBytes memory and the concurrent context-watching
goroutine to close the database still runs, but if it fires, it then
gets blocked on the mutex until the next call to a Rows method (Next,
NextResultSet, Err, Close).
Updates #60304
Updates #53970 (earlier one I'd missed)
Change-Id: Ie41c0c6f32c24887b2f53ec3686c2aab73a1bfff
Reviewed-on: https://go-review.googlesource.com/c/go/+/497675
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
This removes an allocation in Conn.grabConn that, while not super
important, was distracting me when optimizing code elsewhere.
While here, convert an atomic that was forgotten when this package was
earlier updated to use the new Go 1.19 typed atomics.
Change-Id: I4666256b4c0512e2162bd485c389130699f9d5ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/475415
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Similar to CL 385934, rely on waiter trigger instead of the WAIT query
prefix and factor out the common test code.
Fixes#53222
Change-Id: I46efc85ca102b350bb4dbe8e514921e016870ffb
Reviewed-on: https://go-review.googlesource.com/c/go/+/416655
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Goutnik <dgoutnik@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Change-Id: Iee18987c495d1d4bde9da888d454eea8079d3ebc
GitHub-Last-Rev: ff5e01599d
GitHub-Pull-Request: golang/go#52949
Reviewed-on: https://go-review.googlesource.com/c/go/+/406915
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Change-Id: I161072698dad8f184b46c6e627bbfbcb18a1b5c6
GitHub-Last-Rev: 7c8d241280
GitHub-Pull-Request: golang/go#52770
Reviewed-on: https://go-review.googlesource.com/c/go/+/404934
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Make some code more simple.
Change-Id: I801adf0dba5f6c515681345c732dbb907f945419
GitHub-Last-Rev: a505146bac
GitHub-Pull-Request: golang/go#49626
Reviewed-on: https://go-review.googlesource.com/c/go/+/364634
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
[This CL is part of a sequence implementing the proposal #51082.
The design doc is at https://go.dev/s/godocfmt-design.]
Run the updated gofmt, which reformats doc comments,
on the main repository. Vendored files are excluded.
For #51082.
Change-Id: I7332f099b60f716295fb34719c98c04eb1a85407
Reviewed-on: https://go-review.googlesource.com/c/go/+/384268
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
A future change to gofmt will rewrite
// Doc comment.
//
func f()
to
// Doc comment.
func f()
Apply that change preemptively to all doc comments.
For #51082.
Change-Id: I4023e16cfb0729b64a8590f071cd92f17343081d
Reviewed-on: https://go-review.googlesource.com/c/go/+/384259
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
A run of lines that are indented with any number of spaces or tabs
format as a <pre> block. This commit fixes various doc comments
that format badly according to that (standard) rule.
For example, consider:
// - List item.
// Second line.
// - Another item.
Because the - lines are unindented, this is actually two paragraphs
separated by a one-line <pre> block. This CL rewrites it to:
// - List item.
// Second line.
// - Another item.
Today, that will format as a single <pre> block.
In a future release, we hope to format it as a bulleted list.
Various other minor fixes as well, all in preparation for reformatting.
For #51082.
Change-Id: I95cf06040d4186830e571cd50148be3bf8daf189
Reviewed-on: https://go-review.googlesource.com/c/go/+/384257
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Replace the WAIT query prefix with a function callback.
This fixes timing issues when the testing on loaded servers.
Fixes#51208
Change-Id: I5151b397b7066c27ce6bc02c160dde0b584934bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/385934
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Trust: Daniel Theophanes <kardianos@gmail.com>
And then revert the bootstrap cmd directories and certain testdata.
And adjust tests as needed.
Not reverting the changes in std that are bootstrapped,
because some of those changes would appear in API docs,
and we want to use any consistently.
Instead, rewrite 'any' to 'interface{}' in cmd/dist for those directories
when preparing the bootstrap copy.
A few files changed as a result of running gofmt -w
not because of interface{} -> any but because they
hadn't been updated for the new //go:build lines.
Fixes#49884.
Change-Id: Ie8045cba995f65bd79c694ec77a1b3d1fe01bb09
Reviewed-on: https://go-review.googlesource.com/c/go/+/368254
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
In function connectionCleanerRunLocked append to closing slice affects db.freeConns and vise versa. Sometimes valid connections are closed and some invalid not.
Change-Id: I5282f15be3e549533b7d994b17b2060db3c0e7da
GitHub-Last-Rev: b3eb3ab6f4
GitHub-Pull-Request: golang/go#49429
Reviewed-on: https://go-review.googlesource.com/c/go/+/362214
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When drivers return driver.ErrBadConn, no meaningful
information about what the cause of the problem is
returned. Ideally the driver.ErrBadConn would be
always caught with the retry loop, but this is not
always the case. Drivers today must choose between
returning a useful error and use the rety logic.
This allows supporting both.
Fixes#47142
Change-Id: I454573028f041dfdf874eed6c254fb194ccf6d96
Reviewed-on: https://go-review.googlesource.com/c/go/+/333949
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Trust: Ian Lance Taylor <iant@golang.org>
Trust: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fix idle connection reuse so that ConnMaxIdleTime clears down excessive
idle connections.
This now ensures that db.freeConn is ordered by returnedAt and that
connections that have been idle for the shortest period are reused
first.
In addition connectionCleanerRunLocked updates the next check deadline
based on idle and maximum life time information so that we avoid waiting
up to double MaxIdleTime to close connections.
Corrected the calling timer of connectionCleaner.
Fixes#39471
Change-Id: I6d26b3542179ef35aa13e5265a89bc0f08ba7fa1
Reviewed-on: https://go-review.googlesource.com/c/go/+/237337
Reviewed-by: Tamás Gulácsi <tgulacsi78@gmail.com>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Ian Lance Taylor <iant@golang.org>
Change-Id: I519b8021fa79dccc5c0ee79134547491116fc4cc
GitHub-Last-Rev: 48869f5434
GitHub-Pull-Request: golang/go#48071
Reviewed-on: https://go-review.googlesource.com/c/go/+/346231
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Keith Randall <khr@golang.org>
The issue go#46783 correctly diagnosed the context timeout
caused an intermittent failure when the context was canceled
prior to the BeginTx call. However due to the asynchronous nature
of canceling a Tx through a context on fast systems, the tx.Prepare
also succeeded. On slower systems or if a time.Sleep was inserted
between the BeginTx and Prepare, the Prepare would fail.
Resolve this by moving the context cancel after the Prepare.
This will still trigger the deadlock which I tested locally.
In addition, I interspersed multiple time.Sleep calls and the
test still functioned.
Fixes#46852
Change-Id: I9cbf90d3c12b2555493a37799738772b615ae39d
Reviewed-on: https://go-review.googlesource.com/c/go/+/329830
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>
This follows the spelling choices that the Go project has made for English words.
https://github.com/golang/go/wiki/Spelling
Change-Id: Ie7c586d2cf23020cb492cfff58c0831d2d8d3a78
GitHub-Last-Rev: e16a32cd22
GitHub-Pull-Request: golang/go#45442
Reviewed-on: https://go-review.googlesource.com/c/go/+/308291
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
The Go standard library retrofitted context support onto existing APIs
using context.Background and later offered variants that directly
supported user-defined context value specification. This commit makes
that behavior clear in documentation and suggests context-aware
alternatives if the user is looking for one.
An example motivation is supporting code for use in systems that expect
APIs to be cancelable for lifecycle correctness or load
shedding/management reasons, as alluded to in
https://blog.golang.org/context-and-structs.
Updates #44143
Change-Id: I2d7f954ddf9b48264d5ebc8d0007058ff9bddf14
Reviewed-on: https://go-review.googlesource.com/c/go/+/296152
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Jean de Klerk <deklerk@google.com>
Trust: Jean de Klerk <deklerk@google.com>
Run-TryBot: Jean de Klerk <deklerk@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This change allows driver implementations to manage resources in
driver.Connector, e.g. to share the same underlying database handle
between multiple connections. That is, it allows embedded databases
with in-memory backends like SQLite and Genji to safely release the
resources once the sql.DB is closed.
This makes it possible to address oddities with in-memory stores in
SQLite and Genji drivers without introducing too much complexity in
the driver implementations.
See also:
- https://github.com/mattn/go-sqlite3/issues/204
- https://github.com/mattn/go-sqlite3/issues/511
- https://github.com/genjidb/genji/issues/210Fixes#41790
Change-Id: Idbd19763134438ed38288b9d44f16608e4e97fd7
GitHub-Last-Rev: 962c785dfb
GitHub-Pull-Request: golang/go#41710
Reviewed-on: https://go-review.googlesource.com/c/go/+/258360
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
This test contained a data race.
On line 437, db.BeginTx starts a goroutine that runs tx.awaitDone,
which reads tx.keepConnOnRollback.
On line 445, the test writes to tx.keepConnOnRollback.
tx.awaitDone waits on ctx, but because ctx is timeout-based,
there's no ordering guarantee between the write and the read.
The race detector never caught this before
because the context package implementation of Done
contained enough synchronization to make it safe.
That synchronization is not package of the context API or guarantees,
and the first several releases it was not present.
Another commit soon will remove that synchronization,
exposing the latent data race.
To fix the race, emulate a time-based context
using an explicit cancellation-based context.
This gives us enough control to avoid the race.
Change-Id: I103fe9b987b1d4c02e7a20ac3c22a682652128b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/288493
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Found by go vet pass "testinggoroutines".
Change-Id: I6360af2079617b7aa62dcb9bd7254578ca5d1c1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/235527
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Drop error check because errors can be
not only ErrTxDone for tx stmt executions,
and the purpose of the test is just reproducing
deadlock.
Fixes#42259
Change-Id: I9e7105ada1403ec7064dcc1c3385b36893a1c195
Reviewed-on: https://go-review.googlesource.com/c/go/+/266097
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Tx acquires tx.closemu W-lock and then acquires stmt.closemu.W-lock
to fully close the transaction and associated prepared statement.
Stmt query and execution run in reverse ways - acquires
stmt.closemu.R-lock and then acquires tx.closemu.R-lock to grab tx
connection, which may cause deadlock.
Prevent the lock is held around tx.closePrepared to ensure no
deadlock happens.
Fixes#40985
Change-Id: If53909822b87bce11861a6e3035ecb9476d2cd17
Reviewed-on: https://go-review.googlesource.com/c/go/+/250178
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Previously, 2 goroutines were created in OpenDB and a comment in the DB.close() field indicated that they were canceled. Later, session Resetter () was removed, but the comment remained the same. This commit just fixes this message
Change-Id: Ie81026f51d7770e9cf8004818154021f626fb2e8
GitHub-Last-Rev: 38b338a0d1
GitHub-Pull-Request: golang/go#42191
Reviewed-on: https://go-review.googlesource.com/c/go/+/264838
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
When zero or less, maxIdleTime and maxLifetime means unlimited.
Helper function shortestIdleTimeLocked must not return the
minimum of the two until both are verified to be greater
then zero.
Fixes#40841
Change-Id: I1130332baf4ad259cd90c10f4221f5def8510655
Reviewed-on: https://go-review.googlesource.com/c/go/+/248817
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
The prior implementation used the format verb %v which unfortunately
improperly wrapped any underlying scanner errors, and we couldn't use
errors.Is nor errors.As. This change fixes that by using the %w verb.
Added a unit to ensure that both error sub string matching works, but
also that errors.Is works as expected.
Fixes#38099
Change-Id: Iea667041dd8081d961246f77f2542330417292dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/248337
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Opening a connection with Connect should still create a derived
context with a timeout because some clients will not use a timeout
and the connection pool may open a connection asynchronously.
Likewise, if a connection close makes a network operation it should
provide some type of sane timeout for the operation.
Fixes#38185
Change-Id: I9b7ce2996c81c486170dcc84b12672a99610fa27
Reviewed-on: https://go-review.googlesource.com/c/go/+/230438
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Do not cancel rows during test. Only cancel the Tx.
Correct the referenced issue number on the test.
Fixes#38597
Change-Id: I0e8ba1bf2a8ba638d121c9c6938501fec1d5e961
Reviewed-on: https://go-review.googlesource.com/c/go/+/229478
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously the connection pool would only count connections
expired in the background connectionCleaner goroutine towards the
MaxLifetimeClosed stat.
This change increments the stat correctly when checking for
expiry in when acquiring and releasing a connection.
Fixes#38058
Change-Id: Id707ddd40a42a4c38658d5f2931da131647d6c29
GitHub-Last-Rev: 0f205ede43
GitHub-Pull-Request: golang/go#38263
Reviewed-on: https://go-review.googlesource.com/c/go/+/227278
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Brad Fitzpatrick
Daniel Martí
cuiweixie
Jinzhu
Dmitri Goutnik
mstmdev
John Bampton
Ryan Leung
Russ Cox
Daniel Theophanes
Bryan C. Mills
Pavel
Mostafa Solati
Steven Hartland
Cuong Manh Le
Alexander Melentyev
korzhao
Ariel Mashraki
Naman Gera
Elias506
Matt T. Proud
Ivan Trubach
Josh Bleecher Snyder
Hein Khant Zaw
Emmanuel T Odeke
Tzu-Chiao Yeh
Ayzat Sadykov
Shinnosuke Sawada
Tim Möhlmann
Brad Erickson
Kevin Burke
Vivian Liang
Colin