Skip to content

MDEV-40218 lock_rec_unlock_unmodified<CELL>() can release a stale per…#5307

Merged
iMineLink merged 1 commit into
10.11from
MDEV-40218
Jun 30, 2026
Merged

MDEV-40218 lock_rec_unlock_unmodified<CELL>() can release a stale per…#5307
iMineLink merged 1 commit into
10.11from
MDEV-40218

Conversation

@iMineLink

Copy link
Copy Markdown
Contributor

…-cell latch

On a secondary index, lock_rec_unlock_unmodified() drops the cell latch and lock_sys.latch before calling lock_sec_rec_some_has_impl(), then re-acquires lock_sys.latch in shared mode, recomputes the cell address and latches the newly computed cell. A concurrent lock_sys_t::hash_table::resize() (rec_hash grows with the buffer pool) during that window reallocates the cell array, so the cell and its latch move. On success the function returned true while holding the latch of the new cell, but lock_release_on_prepare_try() then released the latch variable it had computed from the old cell address, which could be stale.

Fix: make the cell parameter of lock_rec_unlock_unmodified() an in/out reference so the function reports the cell it currently holds, and have the CELL caller release that cell's latch. This reuses the cell address the function already recomputed, avoiding a second rec_hash lookup.

…-cell latch

On a secondary index, lock_rec_unlock_unmodified<CELL>() drops the cell
latch and lock_sys.latch before calling lock_sec_rec_some_has_impl(), then
re-acquires lock_sys.latch in shared mode, recomputes the cell address and
latches the newly computed cell.  A concurrent lock_sys_t::hash_table::resize()
(rec_hash grows with the buffer pool) during that window reallocates the cell
array, so the cell and its latch move.  On success the function returned true
while holding the latch of the new cell, but lock_release_on_prepare_try() then
released the latch variable it had computed from the old cell address, which
could be stale.

Fix: make the cell parameter of lock_rec_unlock_unmodified() an in/out
reference so the function reports the cell it currently holds, and have the
CELL caller release that cell's latch.  This reuses the cell address the
function already recomputed, avoiding a second rec_hash lookup.
@iMineLink iMineLink requested review from Copilot and dr-m June 30, 2026 11:41
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request modifies lock_rec_unlock_unmodified to pass the cell parameter by reference, allowing it to be updated in place if a concurrent hash table resize occurs. It also updates lock_release_on_prepare_try to release the latch of the updated cell rather than a potentially stale latch. There are no review comments, and the changes appear correct.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a concurrency bug in InnoDB record-lock cleanup on secondary indexes where lock_rec_unlock_unmodified<CELL>() could end up holding a different rec_hash cell latch after a concurrent lock_sys_t::hash_table::resize(), while the caller still released the latch derived from the old (stale) cell address.

Changes:

  • Change lock_rec_unlock_unmodified()’s cell parameter to an in/out reference (hash_cell_t*&) so it can report the currently held (possibly moved) cell back to the caller.
  • Update the CELL call site in lock_release_on_prepare_try() to release the latch corresponding to the updated cell, not the previously computed latch pointer.
  • Expand the parameter documentation to describe the resize/move behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread storage/innobase/lock/lock0lock.cc

@dr-m dr-m left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch. If I understood correctly, the error scenario involves

SET GLOBAL innodb_buffer_pool_size=…;

which used to be more severely broken before #3826 and other changes listed in #4729.

@iMineLink iMineLink merged commit a65241a into 10.11 Jun 30, 2026
16 of 18 checks passed
@iMineLink iMineLink deleted the MDEV-40218 branch June 30, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants