From a3895fe9f18682d5055cb68283eba21d7255a67f Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 23 Jul 2025 14:26:54 -0700 Subject: database/sql: avoid closing Rows while scan is in progress A database/sql/driver.Rows can return database-owned data from Rows.Next. The driver.Rows documentation doesn't explicitly document the lifetime guarantees for this data, but a reasonable expectation is that the caller of Next should only access it until the next call to Rows.Close or Rows.Next. Avoid violating that constraint when a query is cancelled while a call to database/sql.Rows.Scan (note the difference between the two different Rows types!) is in progress. We previously took care to avoid closing a driver.Rows while the user has access to driver-owned memory via a RawData, but we could still close a driver.Rows while a Scan call was in the process of reading previously-returned driver-owned data. Update the fake DB used in database/sql tests to invalidate returned data to help catch other places we might be incorrectly retaining it. Fixes #74831. Change-Id: Ice45b5fad51b679c38e3e1d21ef39156b56d6037 Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/2540 Reviewed-by: Roland Shoemaker Reviewed-by: Neal Patel Reviewed-on: https://go-review.googlesource.com/c/go/+/693735 Auto-Submit: Dmitri Shuralyov Reviewed-by: Roland Shoemaker LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov --- src/database/sql/sql.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) (limited to 'src/database/sql/sql.go') diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index b0abcf7fcd..4be450ca87 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -3368,38 +3368,36 @@ func (rs *Rows) Scan(dest ...any) error { // without calling Next. return fmt.Errorf("sql: Scan called without calling Next (closemuScanHold)") } + rs.closemu.RLock() + rs.raw = rs.raw[:0] + err := rs.scanLocked(dest...) + if err == nil && scanArgsContainRawBytes(dest) { + rs.closemuScanHold = true + } else { + rs.closemu.RUnlock() + } + return err +} +func (rs *Rows) scanLocked(dest ...any) error { if rs.lasterr != nil && rs.lasterr != io.EOF { - rs.closemu.RUnlock() return rs.lasterr } if rs.closed { - err := rs.lasterrOrErrLocked(errRowsClosed) - rs.closemu.RUnlock() - return err - } - - if scanArgsContainRawBytes(dest) { - rs.closemuScanHold = true - rs.raw = rs.raw[:0] - } else { - rs.closemu.RUnlock() + return rs.lasterrOrErrLocked(errRowsClosed) } if rs.lastcols == nil { - rs.closemuRUnlockIfHeldByScan() return errors.New("sql: Scan called without calling Next") } if len(dest) != len(rs.lastcols) { - rs.closemuRUnlockIfHeldByScan() return fmt.Errorf("sql: expected %d destination arguments in Scan, not %d", len(rs.lastcols), len(dest)) } for i, sv := range rs.lastcols { err := convertAssignRows(dest[i], sv, rs) if err != nil { - rs.closemuRUnlockIfHeldByScan() return fmt.Errorf(`sql: Scan error on column index %d, name %q: %w`, i, rs.rowsi.Columns()[i], err) } } -- cgit v1.3