cockroachdb / cockroachdb/cockroach
sqlliveness: deleteOrFetchSession does not reset alive on transaction retry
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
## Summary
`Storage.deleteOrFetchSession` in `pkg/sql/sqlliveness/slstorage/slstorage.go` fails to reset the captured `alive` variable at the start of each transaction retry. If a transaction auto-commit fails with a retryable error after the session was determined to be alive, the retry may delete the now-expired session while still returning `alive=true`.
## Affected Code
**File:** `pkg/sql/sqlliveness/slstorage/slstorage.go`, lines 317-394
The transaction retry function resets captured variables on line 330:
```go
// Reset captured variable in case of retry.
deleted, expiration, prevExpiration = false, hlc.Timestamp{}, hlc.Timestamp{}
```
This reset omits `alive` (and `err`), both of which are named return parameters captured by the closure and modified within it.
## Reproduction Scenario
1. `isAlive` is called for a session, triggering `deleteOrFetchSession`.
2. The transaction reads the session and finds it alive (`alive=true` set at line 369).
3. `txn.exec` auto-commit fails with a `TransactionRetryWithProtoRefreshError`.
4. The closure is retried. Line 330 resets `deleted`, `expiration`, `prevExpiration` but NOT `alive`.
5. On retry, the session has expired. The code deletes the session.
6. The function returns `alive=true, expiration={}, err=nil`.
## Suggested Fix
Add `alive` (and `err`) to the reset on line 330:
```go
alive, deleted, expiration, prevExpiration = false, false, hlc.Timestamp{}, hlc.Timestamp{}
err = nil
```
_This issue was found via automated deep static analysis._
Jira issue: CRDB-62028
Contributor guide
Assessment
This issue has not been assessed yet.