sql: potential nil ctx on connEx.close()

[andreimatei]

Spin off from #51460.

I think there's a bug in the connExecutor closing code. I think it can lead to crashes, although I'm not sure.

Look at what this event does https://github.com/cockroachdb/cockroach/blob/3639ec53c03c9606f187577d684712c6e0bc9427/pkg/sql/conn_fsm.go#L368-L369 This event is generated here when the connExecutor is closing (i.e. when the pgwire connection is terminated). This event leaves the connExecutor in stateAborted but also clears txnState.Ctx (through cleanupAndFinishOnError(). That's not kosher, since connEx.Ctx() expects there to be a non-nil txnState.Ctx whenever the state is not stateNoTxn. Even though the executor is closing, I don't think this is benign. I think we might try to use that nil ctx and crash.

Jira issue: CRDB-4042

[blathers-crl[bot]]

Hi @andreimatei, I've guessed the C-ategory of your issue and suitably labeled it. Please re-label if inaccurate.

While you're here, please consider adding an A- label to help keep our repository tidy.

_{:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[andreimatei]

Another case where the same bug might be triggered is here. If preparing a COMMIT fails, and we're already in the aborted state, we'd generate an eventNonRetriableErr{IsCommit: fsm.True}. That's not what we want here - we want an eventNonRetriableErr{IsCommit: fsm.False}. I don't know if preparing a COMMIT is actually possible. I think PG doesn't allow it; I don't know if we protect against it in any way. In any case, we should fix this.

[github-actions[bot]]

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 10 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!