Skip to content

Commit

Permalink
sql: fix race condition in internal executor
Browse files Browse the repository at this point in the history
In the case where the finish() method was called because of a context
cancellation, the client might not have observed the context error and
instead would have assumed that execution completed successfully. The
code now prioritizes the context error if there is one.

Fixes #62948

Release note: None
  • Loading branch information
ajwerner committed Apr 2, 2021
1 parent a5f2143 commit 010591f
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions pkg/sql/internal_result_channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (c *asyncIEResultChannel) firstResult(
return ieIteratorResult{}, true, ctx.Err()
case res, ok := <-c.dataCh:
if !ok {
return ieIteratorResult{}, true, nil
return ieIteratorResult{}, true, ctx.Err()
}
return res, false, nil
}
Expand Down Expand Up @@ -178,7 +178,7 @@ func (i *syncIEResultChannel) firstResult(
case <-ctx.Done():
return ieIteratorResult{}, true, ctx.Err()
case <-i.doneCh:
return ieIteratorResult{}, true, nil
return ieIteratorResult{}, true, ctx.Err()
case res, ok := <-i.dataCh:
if !ok {
return ieIteratorResult{}, true, nil
Expand Down Expand Up @@ -226,13 +226,21 @@ func (i *syncIEResultChannel) addResult(ctx context.Context, result ieIteratorRe
case <-ctx.Done():
return ctx.Err()
case <-i.doneCh:
// Prefer the context error if there is one.
if ctxErr := ctx.Err(); ctxErr != nil {
return ctxErr
}
return errSyncIEResultReaderCanceled
case i.dataCh <- result:
}
select {
case <-ctx.Done():
return ctx.Err()
case <-i.doneCh:
// Prefer the context error if there is one.
if ctxErr := ctx.Err(); ctxErr != nil {
return ctxErr
}
return errSyncIEResultReaderCanceled
case <-i.waitCh:
return nil
Expand Down

0 comments on commit 010591f

Please sign in to comment.