Skip to content

MDEV-40800 Assert in ha_close() when a temporary table converts - #5565

Open
arcivanov wants to merge 2 commits into
MariaDB:mainfrom
arcivanov:MDEV-40800
Open

MDEV-40800 Assert in ha_close() when a temporary table converts#5565
arcivanov wants to merge 2 commits into
MariaDB:mainfrom
arcivanov:MDEV-40800

Conversation

@arcivanov

Copy link
Copy Markdown
Contributor

Two independent defects in create_internal_tmp_table_from_heap(), one
commit each.

MDEV-40800: PFS batch mode is left on a handler that is about to be deleted

JOIN_CACHE::join_records() puts the handler of its table into PFS batch mode
and only then opens the scan, and it is that scan which materializes a derived
table for the first time. When the temporary table outgrows the in-memory
engine at that point, create_internal_tmp_table_from_heap() closes and
deletes the very handler the batch was started on:

handler::ha_close ... Assertion `m_psi_batch_mode == PSI_BATCH_MODE_NONE' failed

The close is not the only call that breaks. After the swap table->file is the
replacement handler, whose mode is PSI_BATCH_MODE_NONE, so the caller's
end_psi_batch_mode() would assert as well. Ending the batch and leaving it
ended is therefore not sufficient.

The batch is handed over instead: ended on the old handler right before
ha_close(), started again on the replacement once the swap is complete. It is
started at the very end so that the writes the conversion performs itself are
not counted into the reader's batch.

sub_select() is not affected, because it materializes in
join_tab_execution_startup() before it starts its own batch.

This is the approach agreed in MDEV-22104, which is the same assert with the
same stacks. That ticket has an unmerged pull request, #3817, based on 10.5.

The new test main.tmp_table_convert_while_read covers five query shapes that
each reach the conversion while a reader is in progress: a derived table with
DISTINCT, one with GROUP BY, an unmerged derived table, a derived table
filled by UNION, and an ALGORITHM=TEMPTABLE view. Each was confirmed
individually to hit the assert without the fix. Created_tmp_disk_tables is
checked after every query, so that a later change to the size limits cannot
silently leave a query without a conversion.

The replacement table is leaked when the scan of the in-memory table cannot start

The same function creates and opens the on-disk replacement before it starts
the scan that supplies the rows, and the failure exit of that scan returned
immediately, skipping all of the cleanup:

  if (table->file->ha_rnd_init_with_error(1))
    DBUG_RETURN(1);

The replacement was left open with its files on disk, its handler was never
freed, save_proc_info was not restored, and everything allocated on
new_table.mem_root was orphaned, since new_table holds a copy of the
MEM_ROOT taken before those allocations. On a debug build this is not a slow
leak but an abort: safemalloc reports the Aria allocations as lost and ~THD()
fails status_var.local_memory_used == 0.

That exit now goes to a new err_drop label placed between the two existing
ones. err_killed cannot be used, because its ha_rnd_end() asserts that a
scan is in progress and a failed rnd_init leaves none. err2 cannot be used,
because it never drops the replacement table.

The failure is not reachable as the code stands, since the in-memory engine
cannot fail to start a scan, so the new case in main.error_simulation drives
it with a debug injection, placed next to the raise_error injection that
already covers the copy loop of the same function.

Testing

--suite=main,perfschema passes 1853/1853 on a debug build. Both new tests
also pass under --ps-protocol --view-protocol --sp-protocol --cursor-protocol.

This is based on main. MDEV-22104 lists 10.11 and 11.4 as affected, so if an
earlier branch is preferred as the target, I am happy to rebase.

`JOIN_CACHE::join_records()` puts the handler of its table into PFS
batch mode and only then opens the scan, and that scan is what
materializes a derived table for the first time. When the temporary
table outgrows the in-memory engine at that point,
`create_internal_tmp_table_from_heap()` closes and deletes the very
handler the batch was started on, and `handler::ha_close()` asserts
that no batch is in progress.

The close is not the only call that breaks. After the swap
`table->file` is the replacement handler, whose mode is
`PSI_BATCH_MODE_NONE`, so the caller's `end_psi_batch_mode()` would
assert too. Ending the batch and leaving it ended is therefore not a
fix.

Hand the batch over to the replacement: end it on the old handler
right before `ha_close()` and start it again on the new one once the
swap is complete. It is started at the very end so that the writes
performed by the conversion itself are not counted into the reader's
batch.

`sub_select()` is not affected: it materializes in
`join_tab_execution_startup()` before it starts its own batch.
`create_internal_tmp_table_from_heap()` creates and opens the on-disk
replacement table before it starts the scan of the in-memory table
that supplies the rows. When that scan could not be started the
function returned immediately, skipping all of the cleanup: the
replacement was left open with its files on disk, its handler was
never freed, the saved `proc_info` was not restored, and the blocks
allocated on `new_table.mem_root` were orphaned, since `new_table`
holds a copy of the `MEM_ROOT` taken before those allocations.

Route that exit through a new `err_drop` label placed between the two
existing ones. `err_killed` cannot be used because its `ha_rnd_end()`
asserts that a scan is in progress, and `err2` cannot be used because
it never drops the replacement table.

The failure is not reachable as the code stands, since the in-memory
engine cannot fail to start a scan, so the test drives it with a new
debug injection next to the one that already covers the copy loop of
the same function.
@gkodinov gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Aug 18, 2026

@gkodinov gkodinov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! This is a preliminary review.

Please consider merging the two commits, unless there's a specific reason to keep them separate.

Please also rebase to the version Elena mentioned on jira (lowest affected).

@gkodinov gkodinov self-assigned this Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements.

Development

Successfully merging this pull request may close these issues.

2 participants