From 007b40adafba6b2d8183ed3304318a9c863ba535 Mon Sep 17 00:00:00 2001 From: Rex Johnston Date: Tue, 20 Oct 2026 06:22:42 +1300 Subject: [PATCH] MDEV-40012 PQ: end a worker's parallel scan before close() frees m_prebuilt 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. --- .../main/parallel_query_no_chunk.result | 133 ++++++++++++++++++ mysql-test/main/parallel_query_no_chunk.test | 83 +++++++++++ sql/sql_parallel_execution.cc | 38 ++++- storage/innobase/handler/ha_innodb.cc | 8 ++ storage/innobase/handler/handler0pscan.cc | 13 +- storage/innobase/include/handler0pscan.h | 7 +- 6 files changed, 274 insertions(+), 8 deletions(-) create mode 100644 mysql-test/main/parallel_query_no_chunk.result create mode 100644 mysql-test/main/parallel_query_no_chunk.test diff --git a/mysql-test/main/parallel_query_no_chunk.result b/mysql-test/main/parallel_query_no_chunk.result new file mode 100644 index 00000000000..388ed4b7efd --- /dev/null +++ b/mysql-test/main/parallel_query_no_chunk.result @@ -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; diff --git a/mysql-test/main/parallel_query_no_chunk.test b/mysql-test/main/parallel_query_no_chunk.test new file mode 100644 index 00000000000..8af6bfe24ca --- /dev/null +++ b/mysql-test/main/parallel_query_no_chunk.test @@ -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; diff --git a/sql/sql_parallel_execution.cc b/sql/sql_parallel_execution.cc index e141c837c35..ddc35270b59 100644 --- a/sql/sql_parallel_execution.cc +++ b/sql/sql_parallel_execution.cc @@ -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(); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 01227e01303..9ea5fec6b4c 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -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) { diff --git a/storage/innobase/handler/handler0pscan.cc b/storage/innobase/handler/handler0pscan.cc index 216d2bef788..dd009d09831 100644 --- a/storage/innobase/handler/handler0pscan.cc +++ b/storage/innobase/handler/handler0pscan.cc @@ -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); diff --git a/storage/innobase/include/handler0pscan.h b/storage/innobase/include/handler0pscan.h index d2c69125b39..6a9a26e4aa4 100644 --- a/storage/innobase/include/handler0pscan.h +++ b/storage/innobase/include/handler0pscan.h @@ -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