executor: replace Call with CallWithRecover in the close of hash join v1 (pingcap#61868)

close pingcap#60926

Author: xzhangxian1008

pkg/executor/join/hash_join_v1.go

@@ -45,6 +45,9 @@ import (
 	"github.com/pingcap/tidb/pkg/util/memory"
 )
 
+// IsChildCloseCalledForTest is used for test
+var IsChildCloseCalledForTest = false
+
 var (
 	_ exec.Executor = &HashJoinV1Exec{}
 	_ exec.Executor = &NestedLoopApplyExec{}
@@ -114,7 +117,7 @@ type HashJoinV1Exec struct {
 }
 
 // Close implements the Executor Close interface.
-func (e *HashJoinV1Exec) Close() error {
+func (e *HashJoinV1Exec) Close() (err error) {
 	if e.closeCh != nil {
 		close(e.closeCh)
 	}
@@ -138,7 +141,11 @@ func (e *HashJoinV1Exec) Close() error {
 			channel.Clear(e.ProbeWorkers[i].joinChkResourceCh)
 		}
 		e.ProbeSideTupleFetcher.probeChkResourceCh = nil
-		terror.Call(e.RowContainer.Close)
+		util.WithRecovery(func() { err = e.RowContainer.Close() }, func(r any) {
+			if r != nil {
+				err = errors.Errorf("%v", r)
+			}
+		})
 		e.HashJoinCtxV1.SessCtx.GetSessionVars().MemTracker.UnbindActionFromHardLimit(e.RowContainer.ActionSpill())
 		e.waiterWg.Wait()
 	}
@@ -159,7 +166,12 @@ func (e *HashJoinV1Exec) Close() error {
 	if e.stats != nil {
 		defer e.Ctx().GetSessionVars().StmtCtx.RuntimeStatsColl.RegisterStats(e.ID(), e.stats)
 	}
-	err := e.BaseExecutor.Close()
+
+	IsChildCloseCalledForTest = true
+	childErr := e.BaseExecutor.Close()
+	if childErr != nil {
+		return childErr
+	}
 	return err
 }
 

pkg/executor/join/hash_table_v1.go

@@ -22,6 +22,7 @@ import (
 	"unsafe"
 
 	"github.com/pingcap/errors"
+	"github.com/pingcap/failpoint"
 	"github.com/pingcap/tidb/pkg/sessionctx"
 	"github.com/pingcap/tidb/pkg/sessionctx/stmtctx"
 	"github.com/pingcap/tidb/pkg/types"
@@ -535,6 +536,8 @@ func (c *hashRowContainer) Len() uint64 {
 }
 
 func (c *hashRowContainer) Close() error {
+	failpoint.Inject("issue60923", nil)
+
 	defer c.memTracker.Detach()
 	c.chkBuf = nil
 	return c.rowContainer.Close()

pkg/executor/test/issuetest/BUILD.bazel

@@ -8,7 +8,7 @@ go_test(
         "main_test.go",
     ],
     flaky = True,
-    shard_count = 24,
+    shard_count = 25,
     deps = [
         "//pkg/autoid_service",
         "//pkg/config",
@@ -21,6 +21,7 @@ go_test(
         "//pkg/parser/mysql",
         "//pkg/session/types",
         "//pkg/testkit",
+        "//pkg/testkit/testfailpoint",
         "//pkg/util",
         "//pkg/util/dbterror/exeerrors",
         "//pkg/util/memory",

pkg/executor/test/issuetest/executor_issue_test.go

@@ -15,6 +15,7 @@
 package issuetest_test
 
 import (
+	"context"
 	"fmt"
 	"math/rand"
 	"strings"
@@ -32,6 +33,7 @@ import (
 	"github.com/pingcap/tidb/pkg/parser/mysql"
 	sessiontypes "github.com/pingcap/tidb/pkg/session/types"
 	"github.com/pingcap/tidb/pkg/testkit"
+	"github.com/pingcap/tidb/pkg/testkit/testfailpoint"
 	"github.com/pingcap/tidb/pkg/util"
 	"github.com/pingcap/tidb/pkg/util/dbterror/exeerrors"
 	"github.com/pingcap/tidb/pkg/util/memory"
@@ -772,3 +774,35 @@ func TestIssue55881(t *testing.T) {
 			"(select max(value) from (select * from cte union all select * from cte union all select * from aaa where aaa.id > bbb.id)) from bbb;")
 	}
 }
+
+func TestIssue60923(t *testing.T) {
+	store := testkit.CreateMockStore(t)
+	tk := testkit.NewTestKit(t, store)
+
+	tk.MustExec("use test")
+	tk.MustExec("drop table if exists t1")
+	tk.MustExec("drop table if exists t2")
+	tk.MustExec("create table t1 (col0 int, col1 int);")
+	tk.MustExec("create table t2 (col0 int, col1 int);")
+	tk.MustExec("insert into t1 values (0, 10), (1, 10), (2, 10), (3, 10), (4, 10), (5, 10), (6, 10), (7, 10), (8, 10), (9, 10), (10, 10);")
+	tk.MustExec("insert into t2 values (0, 5), (0, 5), (1, 5), (2, 5), (2, 5), (3, 5), (4, 5), (5, 5), (5, 5), (6, 5), (7, 5), (8, 5), (8, 5), (9, 5), (9, 5), (10, 5);")
+
+	testfailpoint.Enable(t, "github.com/pingcap/tidb/pkg/executor/join/issue60923", "panic")
+	tk.MustExec("set tidb_hash_join_version=legacy")
+
+	ctx := context.Background()
+	join.IsChildCloseCalledForTest = false
+	rs, _ := tk.ExecWithContext(context.Background(), "select * from t1 join (select col0, sum(col1) from t2 group by col0) as r on t1.col0 = r.col0;")
+	req := rs.NewChunk(nil)
+	for {
+		err := rs.Next(ctx, req)
+		require.NoError(t, err)
+		if req.NumRows() == 0 {
+			break
+		}
+	}
+	if rs != nil {
+		require.Error(t, rs.Close())
+	}
+	require.True(t, join.IsChildCloseCalledForTest)
+}