cockroachdb / cockroachdb/cockroach
sql: internal executor iterator pattern makes it easy to create invalid uses of txn
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
This issue describes the more general problem around #99171.
The following code pattern is both *very easy to introduce/use* (because it's convenient) and also *extremely invalid* because it violates the Txn API contract:
```go
it, _ := internalExecutor.QueryIteratorEx(... txn1 ...)
defer it.Close()
for hasNext, err = it.Next(ctx); hasNext; hasNext, err = it.Next(ctx) {
// Here the iterator is active, the plan above is running
...
/* EITHER: */
result := internalExecutor.Exec( ... txn1 /* same txn as above */ ... )
/* OR: */
/* do something else with the txn object that issues KV requests */
/* THIS INCLUDES INDIRECT CALLS e.g. via APIs for user authorization / privilege checks */
...
}
```
The following is _also_ problematic because the query plan may have started to execute before the first Next call:
```go
it, _ := internalExecutor.QueryIteratorEx(... txn1 ...)
defer it.Close()
/* EITHER: */
result := internalExecutor.Exec( ... txn1 /* same txn as above */ ... )
/* OR: */
/* do something else with the txn object that issues KV requests */
/* THIS INCLUDES INDIRECT CALLS e.g. via APIs for user authorization / privilege checks */
for hasNext, err = it.Next(ctx); hasNext; hasNext, err = it.Next(ctx) {
// ...
}
```
The issue is that while the iterator is active, the kv.Txn may have some LeafTxn objects active and issuing KV requests. During that time, it is not valid to concurrently use the RootTxn object associated with the kv.Txn -- in particular not via another concurrent use of the internal executor, but also not via other KV interfaces.
This problem already existed in the past but through the v23.1 cycle we have introduced more uses of the iterator pattern, so we have become more sensitive to it.
Jira issue: CRDB-25758
Contributor guide
Assessment
This issue has not been assessed yet.