From 2e11f21cedd4ac23a99bd5b6cb4afc2b50076d58 Mon Sep 17 00:00:00 2001 From: Chao Wang Date: Thu, 2 Feb 2023 18:21:32 +0800 Subject: [PATCH 1/2] *: fix the sysvar value may be corrupted after set by subquery --- executor/set.go | 11 ++++++++++- server/tidb_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/executor/set.go b/executor/set.go index 2396356c1245b..a7b5ff19da91c 100644 --- a/executor/set.go +++ b/executor/set.go @@ -284,7 +284,16 @@ func (e *SetExecutor) getVarValue(ctx context.Context, v *expression.VarAssignme if err != nil || nativeVal.IsNull() { return "", err } - return nativeVal.ToString() + + value, err = nativeVal.ToString() + if err != nil { + return "", err + } + + // We need to clone the string because the value is constructed by `hack.String` in Datum which reuses the under layer `[]byte` + // instead of allocating some new spaces. The `[]byte` in Datum will be reused in `chunk.Chunk` by different statements in session. + // If we do not clone the value, the system variable will have a risk to be modified by other statements. + return strings.Clone(value), nil } func (e *SetExecutor) loadSnapshotInfoSchemaIfNeeded(name string, snapshotTS uint64) error { diff --git a/server/tidb_test.go b/server/tidb_test.go index e8ffadc4fcdfa..46d59e071b8cc 100644 --- a/server/tidb_test.go +++ b/server/tidb_test.go @@ -2950,3 +2950,38 @@ func TestSandBoxMode(t *testing.T) { require.NoError(t, err) } } + +func TestIssue40979(t *testing.T) { + ts := createTidbTestSuite(t) + + db, err := sql.Open("mysql", ts.getDSN()) + require.NoError(t, err) + defer func() { + require.NoError(t, db.Close()) + }() + + conn, err := db.Conn(context.Background()) + require.NoError(t, err) + defer func() { + require.NoError(t, conn.Close()) + }() + + rs, err := conn.QueryContext(context.Background(), "show tables in test") + ts.Rows(t, rs) + + _, err = conn.ExecContext(context.Background(), "set @@time_zone=(select 'Asia/Shanghai')") + require.NoError(t, err) + + rs, err = conn.QueryContext(context.Background(), "select TIDB_TABLE_ID from information_schema.tables where TABLE_SCHEMA='aaaa'") + ts.Rows(t, rs) + + rs, err = conn.QueryContext(context.Background(), "select @@time_zone") + require.NoError(t, err) + defer func() { + require.NoError(t, rs.Close()) + }() + + rows := ts.Rows(t, rs) + require.Equal(t, 1, len(rows)) + require.Equal(t, "Asia/Shanghai", rows[0]) +} From 1d5eecfcd6f50f21ca4f8c913fe963acf123e74c Mon Sep 17 00:00:00 2001 From: Chao Wang Date: Mon, 6 Feb 2023 10:53:51 +0800 Subject: [PATCH 2/2] modify test case name --- server/tidb_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/tidb_test.go b/server/tidb_test.go index 46d59e071b8cc..11c31354ca0e9 100644 --- a/server/tidb_test.go +++ b/server/tidb_test.go @@ -2951,7 +2951,9 @@ func TestSandBoxMode(t *testing.T) { } } -func TestIssue40979(t *testing.T) { +// See: https://github.com/pingcap/tidb/issues/40979 +// Reusing memory of `chunk.Chunk` may cause some systems variable's memory value to be modified unexpectedly. +func TestChunkReuseCorruptSysVarString(t *testing.T) { ts := createTidbTestSuite(t) db, err := sql.Open("mysql", ts.getDSN())