Merge #157773

157773: logictest: fix recently introduced race around allowUnsafe r=yuzefovich a=yuzefovich

Just use the atomic since we might have concurrent internal queries reading the current value of the testing knob override.

Fixes: #157378
Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

pkg/sql/logictest/logic.go

@@ -27,6 +27,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"text/tabwriter"
 	"time"
@@ -1126,7 +1127,7 @@ type logicTest struct {
 
 	// allowUnsafe is a variable which controls whether the test can access
 	// unsafe internals.
-	allowUnsafe bool
+	allowUnsafe atomic.Bool
 }
 
 func (t *logicTest) t() *testing.T {
@@ -1501,7 +1502,10 @@ func (t *logicTest) newCluster(
 			DisableOptimizerRuleProbability: *disableOptRuleProbability,
 			OptimizerCostPerturbation:       *optimizerCostPerturbation,
 			ForceProductionValues:           serverArgs.ForceProductionValues,
-			UnsafeOverride:                  func() *bool { return &t.allowUnsafe },
+			UnsafeOverride: func() *bool {
+				v := t.allowUnsafe.Load()
+				return &v
+			},
 		}
 		knobs.SQLExecutor = &sql.ExecutorTestingKnobs{
 			DeterministicExplain:            true,
@@ -4649,8 +4653,8 @@ func RunLogicTest(
 		perErrorSummary:            make(map[string][]string),
 		rng:                        rng,
 		declarativeCorpusCollector: cc,
-		allowUnsafe:                true,
 	}
+	lt.allowUnsafe.Store(true)
 	if *printErrorSummary {
 		defer lt.printErrorSummary()
 	}
@@ -4940,8 +4944,8 @@ func (t *logicTest) setSafetyGate(sql string, skip bool) func() {
 		return func() {}
 	}
 
-	t.allowUnsafe = false
+	t.allowUnsafe.Store(false)
 	return func() {
-		t.allowUnsafe = true
+		t.allowUnsafe.Store(true)
 	}
 }

pkg/sql/logictest/parallel_test.go

@@ -67,15 +67,15 @@ func (t *parallelTest) processTestFile(path string, nodeIdx int, db *gosql.DB, c
 	// Set up a dummy logicTest structure to use that code.
 	rng, _ := randutil.NewTestRand()
 	l := &logicTest{
-		rootT:       t.T,
-		cluster:     t.cluster,
-		nodeIdx:     nodeIdx,
-		db:          db,
-		user:        username.RootUser,
-		verbose:     testing.Verbose() || log.V(1),
-		rng:         rng,
-		allowUnsafe: true,
+		rootT:   t.T,
+		cluster: t.cluster,
+		nodeIdx: nodeIdx,
+		db:      db,
+		user:    username.RootUser,
+		verbose: testing.Verbose() || log.V(1),
+		rng:     rng,
 	}
+	l.allowUnsafe.Store(true)
 	if err := l.processTestFile(path, logictest.TestClusterConfig{}); err != nil {
 		log.Dev.Errorf(context.Background(), "error processing %s: %s", path, err)
 		t.Error(err)