Skip to content

Commit

Permalink
sql: do not close stmt buffer of internal executor in errCallback
Browse files Browse the repository at this point in the history
Previously, we would close the stmt buffer of the internal executor in
`errCallback`, "just to be safe" since it was assumed that the buffer is
already closed when the callback is executed. The callback runs whenever
`run()` loop of connExecutor exits with an error.

However, it is possible for the following sequence of events to happen:
- the new goroutine is span for the internal executor before any
commands are pushed into the stmt buffer
- the context is canceled before the new goroutine blocks waiting for
the command to execute, i.e. `run()` loop is exited before any commands
are executed
- the `errCallback` with the context cancellation is performed. This
closes the stmt buffer. The goroutine exits
- the main goroutine tries to push some commands into the buffer only to
find that it was already closed. An assertion error is returned, and
a sentry event is created.

I think we should just not close the stmt buffer in the `errCallback`
since this was never necessary and can lead to the scenario described
above where no sentry event should be emitted.

Release note: None
  • Loading branch information
yuzefovich committed Apr 16, 2022
1 parent 771432d commit 8e5f677
Showing 1 changed file with 0 additions and 3 deletions.
3 changes: 0 additions & 3 deletions pkg/sql/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -727,9 +727,6 @@ func (ie *InternalExecutor) execInternal(
// errCallback is called if an error is returned from the connExecutor's
// run() loop.
errCallback := func(err error) {
// The connExecutor exited its run() loop, so the stmtBuf must have been
// closed. Still, since Close() is idempotent, we'll call it here too.
stmtBuf.Close()
_ = rw.addResult(ctx, ieIteratorResult{err: err})
}
ie.initConnEx(ctx, txn, rw, sd, stmtBuf, &wg, syncCallback, errCallback)
Expand Down

0 comments on commit 8e5f677

Please sign in to comment.