cockroachdb / cockroachdb/cockroach

sqlliveness: deleteOrFetchSession does not reset alive on transaction retry

Open
#166,616 1 comment 0 reactions 0 assignees View on GitHub
branch-master C-bug O-agent T-testeng
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.