Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DATA RACE in the stmtctx.(*StatementContext) #55468

Closed
hawkingrei opened this issue Aug 16, 2024 · 2 comments · Fixed by #57012
Closed

DATA RACE in the stmtctx.(*StatementContext) #55468

hawkingrei opened this issue Aug 16, 2024 · 2 comments · Fixed by #57012
Assignees
Labels
affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.5 This bug affects the 8.5.x(LTS) versions. component/test severity/major sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@hawkingrei
Copy link
Member

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

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

3. What did you see instead (Required)

==================
WARNING: DATA RACE
Write at 0x00c0089d8db0 by goroutine 65:
  github.com/pingcap/tidb/pkg/sessionctx/stmtctx.(*StatementContext).Reset()
      pkg/sessionctx/stmtctx/stmtctx.go:466 +0x238
  github.com/pingcap/tidb/pkg/sessionctx/variable.(*SessionVars).InitStatementContext()
      pkg/sessionctx/variable/session.go:1783 +0x79
  github.com/pingcap/tidb/pkg/executor.ResetContextOfStmt()
      pkg/executor/executor.go:1799 +0x537
  github.com/pingcap/tidb/pkg/session.(*session).ExecuteStmt()
      pkg/session/session.go:2015 +0x285
  github.com/pingcap/tidb/pkg/testkit.(*TestKit).ExecWithContext()
      pkg/testkit/testkit.go:423 +0xe29
  github.com/pingcap/tidb/pkg/testkit.(*TestKit).MustQueryWithContext()
      pkg/testkit/testkit.go:234 +0x17b
  github.com/pingcap/tidb/pkg/testkit.(*TestKit).MustQuery()
      pkg/testkit/testkit.go:176 +0x10f
  pkg/executor/test/executor/executor_test.TestLowResolutionTSORead()
      pkg/executor/test/executor/executor_test.go:1931 +0x5b0
  github.com/pingcap/tidb/pkg/sessionctx/variable.parseSchemaCacheSize()
      pkg/sessionctx/variable/varsutil.go:628 +0x17c
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func604()
      pkg/sessionctx/variable/sysvar.go:3195 +0x64
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func603()
      pkg/sessionctx/variable/sysvar.go:3187 +0x58
  github.com/pingcap/tidb/pkg/sessionctx/variable.(*SysVar).ValidateWithRelaxedValidation()
      pkg/sessionctx/variable/variable.go:365 +0x23e
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:144 +0x944
  fmt.Sscanf()
      GOROOT/src/fmt/scan.go:114 +0x18e
  github.com/pingcap/tidb/pkg/sessionctx/variable.parseByteSize()
      pkg/sessionctx/variable/varsutil.go:406 +0x1d
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func252()
      pkg/sessionctx/variable/sysvar.go:1371 +0x34
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  fmt.Sscanf()
      GOROOT/src/fmt/scan.go:114 +0x18e
  github.com/pingcap/tidb/pkg/sessionctx/variable.parseByteSize()
      pkg/sessionctx/variable/varsutil.go:406 +0x1d
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func250()
      pkg/sessionctx/variable/sysvar.go:1356 +0x34
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  github.com/pingcap/tidb/pkg/domain.(*Domain).LoadSysVarCacheLoop()
      pkg/domain/domain.go:1857 +0x93
  github.com/pingcap/tidb/pkg/session.bootstrapSessionImpl()
      pkg/session/session.go:3519 +0x884
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func252()
      pkg/sessionctx/variable/sysvar.go:1371 +0x34
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  fmt.Sscanf()
      GOROOT/src/fmt/scan.go:114 +0x18e
  github.com/pingcap/tidb/pkg/sessionctx/variable.parseByteSize()
      pkg/sessionctx/variable/varsutil.go:406 +0x1d
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func250()
      pkg/sessionctx/variable/sysvar.go:1356 +0x34
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  github.com/pingcap/tidb/pkg/sessionctx/variable.parseSchemaCacheSize()
      pkg/sessionctx/variable/varsutil.go:628 +0x17c
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func604()
      pkg/sessionctx/variable/sysvar.go:3195 +0x64
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func603()
      pkg/sessionctx/variable/sysvar.go:3187 +0x58
  github.com/pingcap/tidb/pkg/sessionctx/variable.(*SysVar).ValidateWithRelaxedValidation()
      pkg/sessionctx/variable/variable.go:365 +0x23e
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:144 +0x944
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCacheIfNeeded()
      pkg/domain/sysvar_cache.go:50 +0x1b5
  github.com/pingcap/tidb/pkg/domain.(*Domain).GetSessionCache()
      pkg/domain/sysvar_cache.go:61 +0x50
  github.com/pingcap/tidb/pkg/session.(*session).loadCommonGlobalVariablesIfNeeded()
      pkg/session/session.go:3896 +0x2bb
  github.com/pingcap/tidb/pkg/session.(*session).ExecuteStmt()
      pkg/session/session.go:2007 +0x184
  github.com/pingcap/tidb/pkg/session.(*session).ExecuteInternal()
      pkg/session/session.go:1518 +0x3b9
  github.com/pingcap/tidb/pkg/domain.(*Domain).LoadPrivilegeLoop()
      pkg/domain/domain.go:1800 +0x108
  github.com/pingcap/tidb/pkg/session.bootstrapSessionImpl()
      pkg/session/session.go:3512 +0x827
  github.com/pingcap/tidb/pkg/session.BootstrapSession()
      pkg/session/session.go:3389 +0xad
  github.com/pingcap/tidb/pkg/testkit.bootstrap()
      pkg/testkit/mockstore.go:254 +0x94
  github.com/pingcap/tidb/pkg/testkit.CreateMockStoreAndDomain()
      pkg/testkit/mockstore.go:225 +0xd2
  github.com/pingcap/tidb/pkg/testkit.CreateMockStore()
      pkg/testkit/mockstore.go:70 +0x3dd
  pkg/executor/test/executor/executor_test.TestLowResolutionTSORead()
      pkg/executor/test/executor/executor_test.go:1884 +0x49
  testing.tRunner()
      GOROOT/src/testing/testing.go:1595 +0x261
  testing.(*T).Run.func1()
      GOROOT/src/testing/testing.go:1648 +0x44
Previous read at 0x00c0089d8db0 by goroutine 12602:
  sync/atomic.LoadInt64()
      src/runtime/race_amd64.s:208 +0xb
  sync/atomic.LoadPointer()
      <autogenerated>:1 +0x10
  github.com/pingcap/tidb/pkg/util/memory.(*Tracker).Consume()
      pkg/util/memory/tracker.go:457 +0x144
  github.com/pingcap/tidb/pkg/store/copr.(*copIteratorWorker).sendToRespCh()
      pkg/store/copr/coprocessor.go:1009 +0x1ec
  github.com/pingcap/tidb/pkg/store/copr.(*copIteratorWorker).handleCopResponse()
      pkg/store/copr/coprocessor.go:1486 +0x24e4
  github.com/pingcap/tidb/pkg/store/copr.(*copIteratorWorker).handleCopPagingResult()
      pkg/store/copr/coprocessor.go:1384 +0xe4
  github.com/pingcap/tidb/pkg/store/copr.(*copIteratorWorker).handleTaskOnce()
      pkg/store/copr/coprocessor.go:1311 +0x2235
  github.com/pingcap/tidb/pkg/store/copr.(*copIteratorWorker).handleTask()
      pkg/store/copr/coprocessor.go:1155 +0x225
  github.com/pingcap/tidb/pkg/store/copr.(*copIteratorWorker).run()
      pkg/store/copr/coprocessor.go:816 +0x1b1
  github.com/pingcap/tidb/pkg/store/copr.(*copIterator).open.func1()
      pkg/store/copr/coprocessor.go:863 +0x4f
Goroutine 65 (running) created at:
  testing.(*T).Run()
      GOROOT/src/testing/testing.go:1648 +0x845
  testing.runTests.func1()
      GOROOT/src/testing/testing.go:2054 +0x84
  testing.tRunner()
      GOROOT/src/testing/testing.go:1595 +0x261
  testing.runTests()
      GOROOT/src/testing/testing.go:2052 +0x8ad
  testing.(*M).Run()
      GOROOT/src/testing/testing.go:1925 +0xcd7
  go.uber.org/goleak.VerifyTestMain()
      external/org_uber_go_goleak/testmain.go:53 +0x64
  pkg/executor/test/executor/executor_test.TestMain()
      pkg/executor/test/executor/main_test.go:49 +0x704
  main.main()
      bazel-out/k8-fastbuild/bin/pkg/executor/test/executor/executor_test_/testmain.go:241 +0x6dc
Goroutine 12602 (running) created at:
  github.com/pingcap/tidb/pkg/store/copr.(*copIterator).open()
      pkg/store/copr/coprocessor.go:863 +0x1b4
  github.com/pingcap/tidb/pkg/store/copr.(*CopClient).Send()
      pkg/store/copr/coprocessor.go:108 +0x376
  github.com/pingcap/tidb/pkg/distsql.Select()
      pkg/distsql/distsql.go:91 +0x879
  github.com/pingcap/tidb/pkg/distsql.SelectWithRuntimeStats()
      pkg/distsql/distsql.go:146 +0x8c
  github.com/pingcap/tidb/pkg/executor.selectResultHook.SelectResult()
      pkg/executor/table_reader.go:70 +0x184
  github.com/pingcap/tidb/pkg/executor.(*TableReaderExecutor).buildResp()
      pkg/executor/table_reader.go:416 +0x8f7
  github.com/pingcap/tidb/pkg/executor.(*TableReaderExecutor).Open()
      pkg/executor/table_reader.go:297 +0xc74
  github.com/pingcap/tidb/pkg/executor/internal/exec.Open()
      pkg/executor/internal/exec/executor.go:428 +0xb8
  github.com/pingcap/tidb/pkg/executor/internal/exec.(*BaseExecutorV2).Open()
      pkg/executor/internal/exec/executor.go:300 +0xb1
  github.com/pingcap/tidb/pkg/executor.(*SelectLockExec).Open()
      pkg/executor/executor.go:1144 +0x233
  github.com/pingcap/tidb/pkg/executor/internal/exec.Open()
      pkg/executor/internal/exec/executor.go:428 +0xb8
  github.com/pingcap/tidb/pkg/executor.(*ExecStmt).openExecutor()
      pkg/executor/adapter.go:1245 +0xf4
  github.com/pingcap/tidb/pkg/executor.(*ExecStmt).Exec()
      pkg/executor/adapter.go:580 +0x11a4
  github.com/pingcap/tidb/pkg/session.runStmt()
      pkg/session/session.go:2284 +0x5ce
  github.com/pingcap/tidb/pkg/session.(*session).ExecuteStmt()
      pkg/session/session.go:2145 +0x1ac4
  github.com/pingcap/tidb/pkg/testkit.(*TestKit).ExecWithContext()
      pkg/testkit/testkit.go:423 +0xe29
  github.com/pingcap/tidb/pkg/testkit.(*TestKit).Exec()
      pkg/testkit/testkit.go:396 +0xb5
  github.com/pingcap/tidb/pkg/testkit.(*TestKit).ExecToErr()
      pkg/testkit/testkit.go:469 +0x6d
  pkg/executor/test/executor/executor_test.TestLowResolutionTSORead()
      pkg/executor/test/executor/executor_test.go:1926 +0x58b
  github.com/pingcap/tidb/pkg/sessionctx/variable.parseSchemaCacheSize()
      pkg/sessionctx/variable/varsutil.go:628 +0x17c
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func604()
      pkg/sessionctx/variable/sysvar.go:3195 +0x64
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func603()
      pkg/sessionctx/variable/sysvar.go:3187 +0x58
  github.com/pingcap/tidb/pkg/sessionctx/variable.(*SysVar).ValidateWithRelaxedValidation()
      pkg/sessionctx/variable/variable.go:365 +0x23e
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:144 +0x944
  fmt.Sscanf()
      GOROOT/src/fmt/scan.go:114 +0x18e
  github.com/pingcap/tidb/pkg/sessionctx/variable.parseByteSize()
      pkg/sessionctx/variable/varsutil.go:406 +0x1d
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func252()
      pkg/sessionctx/variable/sysvar.go:1371 +0x34
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  fmt.Sscanf()
      GOROOT/src/fmt/scan.go:114 +0x18e
  github.com/pingcap/tidb/pkg/sessionctx/variable.parseByteSize()
      pkg/sessionctx/variable/varsutil.go:406 +0x1d
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func250()
      pkg/sessionctx/variable/sysvar.go:1356 +0x34
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  github.com/pingcap/tidb/pkg/domain.(*Domain).LoadSysVarCacheLoop()
      pkg/domain/domain.go:1857 +0x93
  github.com/pingcap/tidb/pkg/session.bootstrapSessionImpl()
      pkg/session/session.go:3519 +0x884
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func252()
      pkg/sessionctx/variable/sysvar.go:1371 +0x34
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  fmt.Sscanf()
      GOROOT/src/fmt/scan.go:114 +0x18e
  github.com/pingcap/tidb/pkg/sessionctx/variable.parseByteSize()
      pkg/sessionctx/variable/varsutil.go:406 +0x1d
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func250()
      pkg/sessionctx/variable/sysvar.go:1356 +0x34
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  github.com/pingcap/tidb/pkg/sessionctx/variable.parseSchemaCacheSize()
      pkg/sessionctx/variable/varsutil.go:628 +0x17c
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func604()
      pkg/sessionctx/variable/sysvar.go:3195 +0x64
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:145 +0x9b4
  github.com/pingcap/tidb/pkg/sessionctx/variable.glob..func603()
      pkg/sessionctx/variable/sysvar.go:3187 +0x58
  github.com/pingcap/tidb/pkg/sessionctx/variable.(*SysVar).ValidateWithRelaxedValidation()
      pkg/sessionctx/variable/variable.go:365 +0x23e
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCache()
      pkg/domain/sysvar_cache.go:144 +0x944
  github.com/pingcap/tidb/pkg/domain.(*Domain).rebuildSysVarCacheIfNeeded()
      pkg/domain/sysvar_cache.go:50 +0x1b5
  github.com/pingcap/tidb/pkg/domain.(*Domain).GetSessionCache()
      pkg/domain/sysvar_cache.go:61 +0x50
  github.com/pingcap/tidb/pkg/session.(*session).loadCommonGlobalVariablesIfNeeded()
      pkg/session/session.go:3896 +0x2bb
  github.com/pingcap/tidb/pkg/session.(*session).ExecuteStmt()
      pkg/session/session.go:2007 +0x184
  github.com/pingcap/tidb/pkg/session.(*session).ExecuteInternal()
      pkg/session/session.go:1518 +0x3b9
  github.com/pingcap/tidb/pkg/domain.(*Domain).LoadPrivilegeLoop()
      pkg/domain/domain.go:1800 +0x108
  github.com/pingcap/tidb/pkg/session.bootstrapSessionImpl()
      pkg/session/session.go:3512 +0x827
  github.com/pingcap/tidb/pkg/session.BootstrapSession()
      pkg/session/session.go:3389 +0xad
  github.com/pingcap/tidb/pkg/testkit.bootstrap()
      pkg/testkit/mockstore.go:254 +0x94
  github.com/pingcap/tidb/pkg/testkit.CreateMockStoreAndDomain()
      pkg/testkit/mockstore.go:225 +0xd2
  github.com/pingcap/tidb/pkg/testkit.CreateMockStore()
      pkg/testkit/mockstore.go:70 +0x3dd
  pkg/executor/test/executor/executor_test.TestLowResolutionTSORead()
      pkg/executor/test/executor/executor_test.go:1884 +0x49
  testing.tRunner()
      GOROOT/src/testing/testing.go:1595 +0x261
  testing.(*T).Run.func1()
      GOROOT/src/testing/testing.go:1648 +0x44
================== 

4. What is your TiDB version? (Required)

@hawkingrei hawkingrei added the type/bug The issue is confirmed as a bug. label Aug 16, 2024
@lcwangchao
Copy link
Collaborator

lcwangchao commented Aug 22, 2024

I think the reason is MemTracker is shared across the statements in a session:

cache struct {
execdetails.RuntimeStatsColl
MemTracker memory.Tracker
DiskTracker disk.Tracker
LogOnExceed [2]memory.LogOnExceed
}

It is introduce to reduce object allocations in PR #26064

However, the background goroutine in coprocessor may still be running after the previous statement finished, and every time a statement starts, StmtCtx will be reset:

func (sc *StatementContext) Reset() {
*sc = StatementContext{
ctxID: contextutil.GenContextID(),
CTEStorageMap: sc.CTEStorageMap,
LockTableIDs: sc.LockTableIDs,
TableStats: sc.TableStats,
MDLRelatedTableIDs: sc.MDLRelatedTableIDs,
TblInfo2UnionScan: sc.TblInfo2UnionScan,
WarnHandler: sc.WarnHandler,
ExtraWarnHandler: sc.ExtraWarnHandler,
IndexUsageCollector: sc.IndexUsageCollector,
}

Does it mean we should not reuse the MemTracker? @tiancaiamao could you take a look?

@tiancaiamao tiancaiamao added affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. and removed may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 may-affects-6.5 may-affects-7.1 may-affects-7.5 may-affects-8.1 labels Oct 30, 2024
@ti-chi-bot ti-chi-bot bot added the affects-8.5 This bug affects the 8.5.x(LTS) versions. label Nov 1, 2024
@ti-chi-bot ti-chi-bot bot closed this as completed in af930be Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.5 This bug affects the 8.5.x(LTS) versions. component/test severity/major sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants