MDEV-40218 lock_rec_unlock_unmodified<CELL>() can release a stale per…#5307
MDEV-40218 lock_rec_unlock_unmodified<CELL>() can release a stale per…#5307iMineLink wants to merge 1 commit into
Conversation
…-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.
|
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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()’scellparameter to an in/out reference (hash_cell_t*&) so it can report the currently held (possibly moved) cell back to the caller. - Update the
CELLcall site inlock_release_on_prepare_try()to release the latch corresponding to the updatedcell, 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.
…-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.