Skip to content

Commit

Permalink
executor: fix goroutine leak when builder panic (pingcap#52471)
Browse files Browse the repository at this point in the history
  • Loading branch information
wshwsh12 authored and 3AceShowHand committed Apr 16, 2024
1 parent 81c1b3d commit 9e08848
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 4 deletions.
25 changes: 21 additions & 4 deletions pkg/executor/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import (
"github.com/pingcap/tidb/pkg/parser/ast"
"github.com/pingcap/tidb/pkg/parser/model"
"github.com/pingcap/tidb/pkg/parser/mysql"
"github.com/pingcap/tidb/pkg/parser/terror"
plannercore "github.com/pingcap/tidb/pkg/planner/core"
plannerutil "github.com/pingcap/tidb/pkg/planner/util"
"github.com/pingcap/tidb/pkg/sessionctx"
Expand Down Expand Up @@ -4500,11 +4501,20 @@ func (builder *dataReaderBuilder) buildProjectionForIndexJoin(
canReorderHandles bool,
memTracker *memory.Tracker,
interruptSignal *atomic.Value,
) (exec.Executor, error) {
childExec, err := builder.buildExecutorForIndexJoinInternal(ctx, v.Children()[0], lookUpContents, indexRanges, keyOff2IdxOff, cwc, canReorderHandles, memTracker, interruptSignal)
) (executor exec.Executor, err error) {
var childExec exec.Executor
childExec, err = builder.buildExecutorForIndexJoinInternal(ctx, v.Children()[0], lookUpContents, indexRanges, keyOff2IdxOff, cwc, canReorderHandles, memTracker, interruptSignal)
if err != nil {
return nil, err
}
defer func() {
if r := recover(); r != nil {
err = util.GetRecoverError(r)
}
if err != nil {
terror.Log(exec.Close(childExec))
}
}()

e := &ProjectionExec{
BaseExecutor: exec.NewBaseExecutor(builder.ctx, v.Schema(), v.ID(), childExec),
Expand All @@ -4519,9 +4529,16 @@ func (builder *dataReaderBuilder) buildProjectionForIndexJoin(
if int64(v.StatsCount()) < int64(builder.ctx.GetSessionVars().MaxChunkSize) {
e.numWorkers = 0
}
failpoint.Inject("buildProjectionForIndexJoinPanic", func(val failpoint.Value) {
if v, ok := val.(bool); ok && v {
panic("buildProjectionForIndexJoinPanic")
}
})
err = e.open(ctx)

return e, err
if err != nil {
return nil, err
}
return e, nil
}

// buildRangesForIndexJoin builds kv ranges for index join when the inner plan is index scan plan.
Expand Down
18 changes: 18 additions & 0 deletions pkg/executor/executor_failpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -699,3 +699,21 @@ func TestHandleForeignKeyCascadePanic(t *testing.T) {
err := tk.ExecToErr("replace into t1 values (1, 2);")
require.ErrorContains(t, err, "handleForeignKeyCascadeError")
}

func TestBuildProjectionForIndexJoinPanic(t *testing.T) {
// Test no goroutine leak.
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t1, t2;")
tk.MustExec("create table t1(a int, b varchar(8));")
tk.MustExec("insert into t1 values(1,'1');")
tk.MustExec("create table t2(a int , b varchar(8) GENERATED ALWAYS AS (c) VIRTUAL, c varchar(8), PRIMARY KEY (a));")
tk.MustExec("insert into t2(a) values(1);")
require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/executor/buildProjectionForIndexJoinPanic", "return(true)"))
defer func() {
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/executor/buildProjectionForIndexJoinPanic"))
}()
err := tk.QueryToErr("select /*+ tidb_inlj(t2) */ t2.b, t1.b from t1 join t2 ON t2.a=t1.a;")
require.ErrorContains(t, err, "buildProjectionForIndexJoinPanic")
}

0 comments on commit 9e08848

Please sign in to comment.