Skip to content

Commit d37fe7f

Browse files
committed
sql: fix node panic on SIGINT during CHECK EXTERNAL CONNECTION
`CHECK EXTERNAL CONNECTION` defers a close of the `rows` channel as part of its execution. It also closes that same channel in its `Close` method. Under normal execution, if `CHECK EXTERNAL CONNECTION` is allowed to complete, that `rows` channel is set to `nil`, so `Close` skips the attempt to close to the channel. However, if a `SIGINT` is sent during the execution to cancel the query, `rows` is never set to `nil` and `Close` will attempt to close a closed channel, causing a node panic. Epic: None Release note (bug fix): Fixed a bug introduced in v25.1.0 that would cause a node panic if a `SIGINT` signal was sent during the execution of a `CHECK EXTERNAL CONNECTION` command.
1 parent 6ad4c58 commit d37fe7f

File tree

1 file changed

+10
-7
lines changed

1 file changed

+10
-7
lines changed

pkg/sql/check_external_connection.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
1414
"github.com/cockroachdb/cockroach/pkg/sql/physicalplan"
1515
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
16+
"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
1617
"github.com/cockroachdb/cockroach/pkg/util/humanizeutil"
1718
"github.com/cockroachdb/cockroach/pkg/util/tracing"
1819
"github.com/cockroachdb/errors"
@@ -26,6 +27,7 @@ func (p *planner) CheckExternalConnection(
2627

2728
type checkExternalConnectionNode struct {
2829
zeroInputPlanNode
30+
execGrp ctxgroup.Group
2931
node *tree.CheckExternalConnection
3032
loc string
3133
params CloudCheckParams
@@ -101,7 +103,9 @@ func (n *checkExternalConnectionNode) startExec(params runParams) error {
101103
return nil
102104
})
103105

104-
go func() {
106+
grp := ctxgroup.WithContext(ctx)
107+
n.execGrp = grp
108+
grp.GoCtx(func(ctx context.Context) error {
105109
recv := MakeDistSQLReceiver(
106110
ctx,
107111
rowWriter,
@@ -117,7 +121,9 @@ func (n *checkExternalConnectionNode) startExec(params runParams) error {
117121
// Copy the eval.Context, as dsp.Run() might change it.
118122
evalCtxCopy := params.extendedEvalCtx.Context.Copy()
119123
dsp.Run(ctx, planCtx, nil, plan, recv, evalCtxCopy, nil /* finishedSetupFn */)
120-
}()
124+
return nil
125+
})
126+
121127
return nil
122128
}
123129

@@ -130,7 +136,6 @@ func (n *checkExternalConnectionNode) Next(params runParams) (bool, error) {
130136
return false, params.ctx.Err()
131137
case row, more := <-n.rows:
132138
if !more {
133-
n.rows = nil
134139
return false, nil
135140
}
136141
n.row = row
@@ -143,10 +148,8 @@ func (n *checkExternalConnectionNode) Values() tree.Datums {
143148
}
144149

145150
func (n *checkExternalConnectionNode) Close(_ context.Context) {
146-
if n.rows != nil {
147-
close(n.rows)
148-
n.rows = nil
149-
}
151+
_ = n.execGrp.Wait()
152+
n.rows = nil
150153
}
151154

152155
func (n *checkExternalConnectionNode) parseParams(params runParams) error {

0 commit comments

Comments
 (0)