Skip to content

Commit 20aebbe

Browse files
committed
distsql: harden recent memory leak fix
In 1b23422 we missed one error code path where we'd not call the context cancellation function, and it's now fixed by allocating the function only when we create the flow. Release note: None
1 parent 14f6534 commit 20aebbe

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

pkg/sql/distsql/server.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -653,10 +653,6 @@ func (ds *ServerImpl) SetupFlow(
653653
// Note: the passed context will be canceled when this RPC completes, so we
654654
// can't associate it with the flow since it outlives the RPC.
655655
ctx = ds.AnnotateCtx(context.Background())
656-
// Ensure that the flow respects the node being shut down. We can only call
657-
// the cancellation function once the flow exits.
658-
var cancel context.CancelFunc
659-
ctx, cancel = ds.Stopper.WithCancelOnQuiesce(ctx)
660656
if err := func() error {
661657
// Reserve some memory for this remote flow which is a poor man's
662658
// admission control based on the RAM usage.
@@ -665,6 +661,14 @@ func (ds *ServerImpl) SetupFlow(
665661
if err != nil {
666662
return err
667663
}
664+
// Ensure that the flow respects the node being shut down. We can only
665+
// call the cancellation function once the flow exits.
666+
//
667+
// setupFlow will either call 'cancel' if an error is returned, or the
668+
// cancellation function is taken over by the flow, and it'll be called
669+
// in Flow.Cleanup.
670+
var cancel context.CancelFunc
671+
ctx, cancel = ds.Stopper.WithCancelOnQuiesce(ctx)
668672
var f flowinfra.Flow
669673
ctx, f, _, err = ds.setupFlow(
670674
ctx, rpcSpan, ds.memMonitor, &reserved, req, nil, /* rowSyncFlowConsumer */

0 commit comments

Comments
 (0)