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

Open
hawkingrei opened this issue Aug 16, 2024 · 2 comments
Open

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

hawkingrei opened this issue Aug 16, 2024 · 2 comments

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants