database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2026-10-23 18:49:29 +03:00
Code Activity

MDEV-40012 PQ: end a worker's parallel scan before close() frees m_prebuilt

Browse Source 
A worker's handler was destroyed with its parallel scan still open.
exec() and scan_only() jump to their exit labels when
parallel_init_worker() fails, and the commonest failure is
HA_ERR_END_OF_FILE: no chunk was left for this worker, which happens all
the time on small tables. InnoDB has allocated m_pscan_worker by then, so
the scan was left for ~ha_innobase() -> parallel_scan_free(), which runs
after close() has freed m_prebuilt. parallel_scan_free() says it drops the
worker without end() for exactly that reason, but ~Parallel_scan_worker()
called end() anyway, and end() writes m_prebuilt->pscan_chunk_clamp: a
heap-use-after-free that ASAN reports on every such worker. In a build
without ASAN the write lands in freed memory, and it is the likely cause of
the intermittent SIGSEGV in free() inside pthread_create() when a later
worker reused a cached thread stack whose DTV had been overwritten.

Three changes close it. Both workers now end the scan at their exit label
instead of after the loop, so every way out ends it while the handler is
still open; ending a scan that was never begun is harmless.
ha_innobase::close() ends any worker or coordinator scan still open before
row_prebuilt_free(), so a path that misses it is ended properly rather than
left for the destructor; with the SQL layer put back as it was, this alone
keeps ASAN clean. And ~Parallel_scan_worker() no longer calls end(), so
parallel_scan_free(), now reached only for a handler destroyed without
close(), cannot write through m_prebuilt either. The close() hook and the
single call site are taken from a patch by a co-worker.

Two debug assertions state the rule the defect broke, so it no longer
takes ASAN to see it. ha_innobase::close() asserts that the SQL layer has
already ended a worker's scan, and parallel_scan_free() asserts that no
worker survives close(). The injection pwt_leave_scan_for_close makes the
SQL layer leave the scan of a worker that holds no cursor -- one handed no
chunk -- for close() to end, so the backstop is exercised on purpose and
the first assertion is told to allow it. pwt_end_worker_scan() holds the
injection and ends the scan at both exit labels.

parallel_query_no_chunk (debug only) runs a full scan and a range scan of a
one-page table with four workers, so three of them get no chunk, twice
each, through the normal and the scan-only path, with and without the
injection, checking the rows and that the workers ran every query. In a
plain debug build, removing the exit-label calls fails the first assertion
and removing the close() backstop fails the second.
~Parallel_scan_worker() no longer calling end() is defence for release
builds and is not reachable while the backstop is there.

This commit was prepared with Claude Code (Opus 5.5), which reproduced the
pthread_create() crash under ASAN and traced it, wrote the fix, folded in
the co-worker's close() hook and single call site, added the debug
assertions, the injection and the test, confirmed in a plain debug build
that the test fails with each of the two fixes removed, and ran the
parallel_query tests in normal and --ps-protocol modes and the main suite
with --parallel-worker-threads=4 under ASAN.
This commit is contained in:
Rex Johnston
2026-10-20 06:22:42 +13:00
parent a3c53f08ec
commit 007b40adaf
6 changed files with 274 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

133
mysql-test/main/parallel_query_no_chunk.result Normal file
View File

@@ -0,0 +1,133 @@
CREATE TABLE t1 (a INT PRIMARY KEY, b INT) ENGINE=InnoDB;
INSERT INTO t1 VALUES (1,10),(2,20),(3,30);
SET @save_dbug= @@global.debug_dbug;
# serial answers
SET SESSION parallel_worker_threads=0;
SELECT a, b FROM t1 WHERE b > 0;
a b
1 10
2 20
3 30
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
a b
2 20
3 30
# the workers end their own scans
SET GLOBAL debug_dbug= @save_dbug;
SET SESSION parallel_worker_threads=4;
FLUSH STATUS;
SELECT a, b FROM t1 WHERE b > 0;
a b
1 10
2 20
3 30
FLUSH STATUS;
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
a b
2 20
3 30
# and again, on the handlers the first runs left behind
FLUSH STATUS;
SELECT a, b FROM t1 WHERE b > 0;
a b
1 10
2 20
3 30
FLUSH STATUS;
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
a b
2 20
3 30
# all four ran in the workers (expect 4)
ran_in_the_workers
4
# the same, in the scan-only path
SET GLOBAL debug_dbug= "+d,pwt_scan_only";
SET SESSION parallel_worker_threads=4;
FLUSH STATUS;
SELECT a, b FROM t1 WHERE b > 0;
a b
1 10
2 20
3 30
FLUSH STATUS;
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
a b
2 20
3 30
# and again, on the handlers the first runs left behind
FLUSH STATUS;
SELECT a, b FROM t1 WHERE b > 0;
a b
1 10
2 20
3 30
FLUSH STATUS;
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
a b
2 20
3 30
# all four ran in the workers (expect 4)
ran_in_the_workers
4
# a worker with no chunk leaves its scan for close() to end
SET GLOBAL debug_dbug= "+d,pwt_leave_scan_for_close";
SET SESSION parallel_worker_threads=4;
FLUSH STATUS;
SELECT a, b FROM t1 WHERE b > 0;
a b
1 10
2 20
3 30
FLUSH STATUS;
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
a b
2 20
3 30
# and again, on the handlers the first runs left behind
FLUSH STATUS;
SELECT a, b FROM t1 WHERE b > 0;
a b
1 10
2 20
3 30
FLUSH STATUS;
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
a b
2 20
3 30
# all four ran in the workers (expect 4)
ran_in_the_workers
4
# the same, in the scan-only path
SET GLOBAL debug_dbug= "+d,pwt_scan_only,pwt_leave_scan_for_close";
SET SESSION parallel_worker_threads=4;
FLUSH STATUS;
SELECT a, b FROM t1 WHERE b > 0;
a b
1 10
2 20
3 30
FLUSH STATUS;
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
a b
2 20
3 30
# and again, on the handlers the first runs left behind
FLUSH STATUS;
SELECT a, b FROM t1 WHERE b > 0;
a b
1 10
2 20
3 30
FLUSH STATUS;
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
a b
2 20
3 30
# all four ran in the workers (expect 4)
ran_in_the_workers
4
SET GLOBAL debug_dbug= @save_dbug;
SET SESSION parallel_worker_threads=default;
DROP TABLE t1;

83
mysql-test/main/parallel_query_no_chunk.test Normal file
View File

@@ -0,0 +1,83 @@
# MDEV-40012 Parallel Query: workers that are handed no chunk of the scan at all
--source include/have_innodb.inc
--source include/have_debug.inc
--source include/not_embedded.inc
CREATE TABLE t1 (a INT PRIMARY KEY, b INT) ENGINE=InnoDB;
INSERT INTO t1 VALUES (1,10),(2,20),(3,30);
SET @save_dbug= @@global.debug_dbug;
--echo # serial answers
SET SESSION parallel_worker_threads=0;
--sorted_result
SELECT a, b FROM t1 WHERE b > 0;
--sorted_result
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
--let $mode= 0
while ($mode < 4)
{
if ($mode == 0)
{
--echo # the workers end their own scans
SET GLOBAL debug_dbug= @save_dbug;
}
if ($mode == 1)
{
--echo # the same, in the scan-only path
SET GLOBAL debug_dbug= "+d,pwt_scan_only";
}
if ($mode == 2)
{
--echo # a worker with no chunk leaves its scan for close() to end
SET GLOBAL debug_dbug= "+d,pwt_leave_scan_for_close";
}
if ($mode == 3)
{
--echo # the same, in the scan-only path
SET GLOBAL debug_dbug= "+d,pwt_scan_only,pwt_leave_scan_for_close";
}
SET SESSION parallel_worker_threads=4;
--let $ran= 0
--let $q= 0
while ($q < 4)
{
if ($q == 2)
{
--echo # and again, on the handlers the first runs left behind
}
FLUSH STATUS;
--sorted_result
if ($q == 0)
{
SELECT a, b FROM t1 WHERE b > 0;
}
--sorted_result
if ($q == 1)
{
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
}
--sorted_result
if ($q == 2)
{
SELECT a, b FROM t1 WHERE b > 0;
}
--sorted_result
if ($q == 3)
{
SELECT a, b FROM t1 WHERE a BETWEEN 2 AND 3;
}
--let $pq= query_get_value(SHOW SESSION STATUS LIKE 'Parallel_queries_executed', Value, 1)
--let $ran= `SELECT $ran + ($pq > 0)`
--inc $q
}
--echo # all four ran in the workers (expect 4)
--disable_query_log
--eval SELECT $ran AS ran_in_the_workers
--enable_query_log
--inc $mode
}
SET GLOBAL debug_dbug= @save_dbug;
SET SESSION parallel_worker_threads=default;
DROP TABLE t1;

38
sql/sql_parallel_execution.cc
View File

@@ -2230,6 +2230,32 @@ int pwt_worker::emit_scanned_row()
}
/**
@brief
End this worker's parallel scan of its copy of the driving table.
@description
Called on every way out of a worker, at the exit label, so a failed
parallel_init_worker() -- "no chunk left" among its failures -- still
has its scan ended while the handler is open. Left for the handler, the
scan is ended by ha_innobase::close(), which a debug build asserts the
SQL layer never relies on.
pwt_leave_scan_for_close leaves that to close() for a worker that holds
no cursor, which is the worker that was handed no chunk: the shape this
function exists for, and the one close() and the destructor have to
survive. A worker that did scan still has its cursor open, and unlocking
the table with it open would assert first.
*/
static void pwt_end_worker_scan(TABLE *src)
{
if (src->file->inited == handler::NONE &&
DBUG_IF("pwt_leave_scan_for_close"))
return;
src->file->parallel_end_worker();
}
/**
@brief
Scan-only PROTOTYPE: read this worker's chunks and ship every row.
@@ -2297,7 +2323,6 @@ int pwt_worker::execute_scan_only()
break;
}
}
src->file->parallel_end_worker();
if (!err && !killed)
sink->flush();
@@ -2311,6 +2336,13 @@ scan_exit:
*/
if (err == HA_ERR_END_OF_FILE)
err= 0;
/*
Ended here, on every way out, rather than after the loop: a failed
parallel_init_worker() -- "no chunk left" among its failures -- jumps
straight to this label, and the engine may already have set the scan up.
Ending one that was never begun is harmless.
*/
pwt_end_worker_scan(src);
src->file->ha_index_or_rnd_end();
src->file->ha_external_lock(thd, F_UNLCK);
DBUG_RETURN(err);
@@ -2441,7 +2473,6 @@ int pwt_worker::execute_and_handoff()
my_error(ER_QUERY_INTERRUPTED, MYF(0));
killed= thd->killed;
}
src->file->parallel_end_worker();
/*
Close the container's index before the transport is told this producer is
@@ -2473,6 +2504,9 @@ exec_exit:
if (err == HA_ERR_END_OF_FILE)
err= 0;
/* Ended here, on every way out, as in the scan-only path; see there. */
pwt_end_worker_scan(src);
// end any open index/rnd scans (no-op for tables left in NONE state), unlock
for (i= 1; i < nt; i++)
exec.jointabs[i].table->file->ha_index_or_rnd_end();

8
storage/innobase/handler/ha_innodb.cc
View File

@@ -6503,6 +6503,14 @@ ha_innobase::close()
{
DBUG_ENTER("ha_innobase::close");
/* Ending a parallel scan uses m_prebuilt, so it must happen before
m_prebuilt is freed. Normally the SQL layer has ended it already and
these do nothing; a debug build insists on that for a worker, unless
the test that exercises this backstop asked for it to be needed. */
DBUG_ASSERT(!m_pscan_worker || DBUG_IF("pwt_leave_scan_for_close"));
parallel_end_worker();
parallel_end_coordinator();
row_prebuilt_free(m_prebuilt);
if (m_upd_buf != NULL) {

13
storage/innobase/handler/handler0pscan.cc
View File

@@ -645,11 +645,14 @@ int ha_innobase::parallel_end_worker()
void ha_innobase::parallel_scan_free()
{
/* Last resort, for a handler torn down without the SQL layer having
ended the scan. The worker is dropped without end(): that restores
state in m_prebuilt, which close() has already freed by now, and the
worker owns no memory of its own. The coordinator does own memory, and
its destructor frees it without touching the handler. */
/* Last resort, for a handler destroyed without close(), which ends any
scan still open. The worker is dropped without end(): that restores
state in m_prebuilt, which may not be there to restore, and the worker
owns no memory of its own. The coordinator does own memory, and its
destructor frees it without touching the handler.
A worker still here after close() is one close() failed to end: the
path that used to write through the freed m_prebuilt. */
DBUG_ASSERT(!m_pscan_worker);
UT_DELETE(m_pscan_worker);
m_pscan_worker = nullptr;
UT_DELETE(m_pscan_coord);

7
storage/innobase/include/handler0pscan.h
View File

@@ -204,7 +204,12 @@ class Parallel_scan_worker
public:
Parallel_scan_worker(ha_innobase *owner) : m_owner(owner) {}
~Parallel_scan_worker() { end(); }
/** Does not call end(): end() restores m_prebuilt, and a worker deleted
without being ended first is being deleted from
ha_innobase::parallel_scan_free(), in the handler's destructor, after
close() has freed m_prebuilt. close() ends any open scan itself, so that
path should find nothing left to delete. */
~Parallel_scan_worker() = default;
/** Take the first chunk and get ready to read it.
@param wctx this worker's context, from