small probability recursive lock (deadlock) when setting tidb_restricted_read_only

[time-and-fate]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

count=0
while true; do
mysql -h 127.0.0.1 -P 4000 -u root -e "set global tidb_restricted_read_only = 1;set global tidb_restricted_read_only = 0;";
count=$((count+1))
echo "$count"
done

I tried several times, in most times it can be triggered before the count reaches 5000.

2. What did you expect to see? (Required)

Execute and finish successfully.

3. What did you see instead (Required)

The SET statement above blocks forever, and any following SET GLOBAL statements will block forever.

4. What is your TiDB version? (Required)

I'm using v8.2.0 and master, but I believe many versions have this bug.

[time-and-fate]

Stack trace:

goroutine 261379 [sync.Mutex.Lock, 1 minutes]:
sync.runtime_SemacquireMutex(0x1e31445?, 0x60?, 0x5ef45e0?)
	/usr/local/go/src/runtime/sema.go:77 +0x25
sync.(*Mutex).lockSlow(0xc0049ed8c8)
	/usr/local/go/src/sync/mutex.go:171 +0x15d
sync.(*Mutex).Lock(...)
	/usr/local/go/src/sync/mutex.go:90
github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache(0xc0049ed800, {0x0, 0x0})
	/workspace/source/tidb/pkg/domain/sysvar_cache.go:120 +0x17d
github.com/pingcap/tidb/pkg/domain.(*Domain).NotifyUpdateSysVarCache(0xc0049ed800?, 0x1?)
	/workspace/source/tidb/pkg/domain/domain.go:2656 +0x235
github.com/pingcap/tidb/pkg/session.(*session).replaceGlobalVariablesTableValue(0xc003a08900, {0x6c1e6a8?, 0xa2803e0?}, {0x619bc3d, 0x14}, {0x6bc4ac0, 0x2}, 0xd8?)
	/workspace/source/tidb/pkg/session/session.go:1287 +0x1c5
github.com/pingcap/tidb/pkg/session.(*session).SetGlobalSysVar(0xc003a08900, {0x6c1e590, 0xa2803e0}, {0x619bc3d, 0x14}, {0x6bc4ac0, 0x2})
	/workspace/source/tidb/pkg/session/session.go:1349 +0x1ef
github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func164({0xc0095080e0?, 0xc00eccc000?}, 0xc00eccc000, {0x6bc4ac0, 0x2})
	/workspace/source/tidb/pkg/sessionctx/variable/sysvar.go:825 +0xcf
github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache(0xc0049ed800, {0x0, 0x0})
	/workspace/source/tidb/pkg/domain/sysvar_cache.go:146 +0x6ee
github.com/pingcap/tidb/pkg/domain.(*Domain).NotifyUpdateSysVarCache(0xc0049ed800?, 0x1?)
	/workspace/source/tidb/pkg/domain/domain.go:2656 +0x235
github.com/pingcap/tidb/pkg/session.(*session).replaceGlobalVariablesTableValue(0xc002c6c480, {0x6c1e6a8?, 0xa2803e0?}, {0x61b7f50, 0x19}, {0x6bc4ac0, 0x2}, 0x0?)
	/workspace/source/tidb/pkg/session/session.go:1287 +0x1c5
github.com/pingcap/tidb/pkg/session.(*session).SetGlobalSysVar(0xc002c6c480, {0x6c1e5c8, 0xc007b59a40}, {0xc00eb1ecfc, 0x19}, {0xc010d5c747, 0x1})
	/workspace/source/tidb/pkg/session/session.go:1349 +0x1ef
github.com/pingcap/tidb/pkg/executor.(*SetExecutor).setSysVariable(0xc006a0da00, {0x6c1e5c8, 0xc007b59a40}, {0xc00eb1ecfc, 0x19}, 0xc007b599e0)
	/workspace/source/tidb/pkg/executor/set.go:155 +0x463
github.com/pingcap/tidb/pkg/executor.(*SetExecutor).Next(0xc006a0da00, {0x6c1e5c8, 0xc007b59a40}, 0xc0147e8e10?)
	/workspace/source/tidb/pkg/executor/set.go:105 +0x3f7
github.com/pingcap/tidb/pkg/executor/internal/exec.Next({0x6c1e5c8, 0xc007b59a40}, {0x6c4ae80, 0xc006a0da00}, 0xc0147e8e10)
	/workspace/source/tidb/pkg/executor/internal/exec/executor.go:410 +0x2da
github.com/pingcap/tidb/pkg/executor.(*ExecStmt).next(0xc00159db30, {0x6c1e5c8, 0xc007b59a40}, {0x6c4ae80, 0xc006a0da00}, 0x0?)
	/workspace/source/tidb/pkg/executor/adapter.go:1241 +0x6e
github.com/pingcap/tidb/pkg/executor.(*ExecStmt).handleNoDelayExecutor(0xc00159db30, {0x6c1e5c8?, 0xc007b59a40?}, {0x6c4ae80?, 0xc006a0da00})
	/workspace/source/tidb/pkg/executor/adapter.go:990 +0x3a9
github.com/pingcap/tidb/pkg/executor.(*ExecStmt).handleNoDelay(0xc00159db30, {0x6c1e5c8, 0xc007b59a40}, {0x6c4ae80?, 0xc006a0da00?}, 0x0)
	/workspace/source/tidb/pkg/executor/adapter.go:824 +0x252
github.com/pingcap/tidb/pkg/executor.(*ExecStmt).Exec(0xc00159db30, {0x6c1e5c8, 0xc007b59a40})
	/workspace/source/tidb/pkg/executor/adapter.go:588 +0xd65
github.com/pingcap/tidb/pkg/session.runStmt({0x6c1e5c8?, 0xc007b59710?}, 0xc002c6c480, {0x6c2da40?, 0xc00159db30})
	/workspace/source/tidb/pkg/session/session.go:2289 +0x322
github.com/pingcap/tidb/pkg/session.(*session).ExecuteStmt(0xc002c6c480, {0x6c1e5c8?, 0xc007b59710?}, {0x6c38578, 0xc003091440?})
	/workspace/source/tidb/pkg/session/session.go:2150 +0x10f4
github.com/pingcap/tidb/pkg/server.(*TiDBContext).ExecuteStmt(0xc00dc76e88, {0x6c1e5c8, 0xc007b59710}, {0x6c38578?, 0xc003091440})
	/workspace/source/tidb/pkg/server/driver_tidb.go:294 +0xa7
github.com/pingcap/tidb/pkg/server.(*clientConn).handleStmt(0xc01411aea0, {0x6c1e600, 0xc0147e8cd0}, {0x6c38578?, 0xc003091440}, {0x0, 0x0, 0x0}, 0x1)
	/workspace/source/tidb/pkg/server/conn.go:2031 +0x2d2
github.com/pingcap/tidb/pkg/server.(*clientConn).handleQuery(0xc01411aea0, {0x6c1e600, 0xc0147e8cd0}, {0xc00eb1ecf1, 0x28})
	/workspace/source/tidb/pkg/server/conn.go:1785 +0xb5e
github.com/pingcap/tidb/pkg/server.(*clientConn).dispatch(0xc01411aea0, {0x6c1e5c8, 0xc00c2cbb60}, {0xc00eb1ecf0, 0x29, 0x29})
	/workspace/source/tidb/pkg/server/conn.go:1359 +0xf2b
github.com/pingcap/tidb/pkg/server.(*clientConn).Run(0xc01411aea0, {0x6c1e5c8, 0xc00c2cbb60})
	/workspace/source/tidb/pkg/server/conn.go:1125 +0x545
github.com/pingcap/tidb/pkg/server.(*Server).onConn(0xc0013e6800?, 0xc01411aea0)
	/workspace/source/tidb/pkg/server/server.go:739 +0x89d
created by github.com/pingcap/tidb/pkg/server.(*Server).startNetworkListener in goroutine 8119
	/workspace/source/tidb/pkg/server/server.go:557 +0x78d

[time-and-fate]

It's trying to recursively get the same lock in rebuildSysVarCache().

The problem mainly comes from here:

tidb/pkg/sessionctx/variable/sysvar.go

Lines 822 to 831 in 2a731e6

	{Scope: ScopeGlobal, Name: TiDBRestrictedReadOnly, Value: BoolToOnOff(DefTiDBRestrictedReadOnly), Type: TypeBool, SetGlobal: func(_ context.Context, s *SessionVars, val string) error {
	on := TiDBOptOn(val)
	// For user initiated SET GLOBAL, also change the value of TiDBSuperReadOnly
	if on && s.StmtCtx.StmtType == "Set" {
	err := s.GlobalVarsAccessor.SetGlobalSysVar(context.Background(), TiDBSuperReadOnly, "ON")
	if err != nil {
	return err
	}
	}
	RestrictedReadOnly.Store(on)

This will potentially cause such a call stack:

SetGlobalSysVar() -> replaceGlobalVariablesTableValue() -> NotifyUpdateSysVarCache() -> rebuildSysVarCache() -> tidb_restricted_read_only SetGlobal() -> SetGlobalSysVar() -> replaceGlobalVariablesTableValue() -> NotifyUpdateSysVarCache() -> rebuildSysVarCache()

Usually, this actually will not happen. Because in rebuildSysVarCache() -> tidb_restricted_read_only SetGlobal(), the SessionVars passed into SetGlobal() is from the system session pool, so s.StmtCtx.StmtType == "Set" will likely be false.

However, the session from the system session pool will contain information of the last executed internal SQL. If that SQL happens to be a SET statement, this deadlock will happen.

[hawkingrei]

it is found by the UT TestAutoCommitRespectsReadOnly in the CI.

#53822

[hawkingrei]

ref #48178