Skip to content

Commit 7aed5eb

Browse files
committed
MDEV-24642 Assertion r->emplace... failed in sux_lock::s_lock_register()
In commit 03ca649 (MDEV-24142) we replaced a debug data structure that holds information about S-latch holders with a std::set, which does not allow duplicates. The assertion failed in btr_search_guess_on_hash() in an s_lock_try() operation. The reason why recursive S-latch requests are not normally allowed is that if some other thread has enqueued a waiting X-lock, then further S-latch requests will block until the exclusive lock has been granted and released. If a thread were already holding one S-latch while waiting for the X-latch to be granted and released by another thread, the two threads would deadlock. However, the nonblocking s_lock_try() is perfectly fine; it will immediately return failure in case of conflict. sux_lock::readers: Use std::unordered_multiset instead of std::set. sux_lock::s_lock_register(): Allow 'duplicate' requests. Blocking-mode latch acquisitions are already covered by !have_s() assertions. sux_lock::s_unlock(): Erase only one element from readers. buf_page_try_get(): Revert to s_lock_try(). It had been previously changed to the more intrusive u_lock_try() in response to the debug check failing.
1 parent e9fc610 commit 7aed5eb

File tree

3 files changed

+15
-14
lines changed

3 files changed

+15
-14
lines changed

storage/innobase/buf/buf0buf.cc

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3434,12 +3434,12 @@ buf_page_optimistic_get(
34343434
return(TRUE);
34353435
}
34363436

3437-
/** Try to U-latch a page.
3437+
/** Try to S-latch a page.
34383438
Suitable for using when holding the lock_sys latches (as it avoids deadlock).
34393439
@param[in] page_id page identifier
34403440
@param[in,out] mtr mini-transaction
34413441
@return the block
3442-
@retval nullptr if an U-latch cannot be granted immediately */
3442+
@retval nullptr if an S-latch cannot be granted immediately */
34433443
buf_block_t *buf_page_try_get(const page_id_t page_id, mtr_t *mtr)
34443444
{
34453445
ut_ad(mtr);
@@ -3461,16 +3461,13 @@ buf_block_t *buf_page_try_get(const page_id_t page_id, mtr_t *mtr)
34613461
buf_block_buf_fix_inc(block);
34623462
hash_lock->read_unlock();
34633463

3464-
/* We will always try to acquire an U latch.
3465-
In lock_rec_print() we may already be holding an S latch on the page,
3466-
and recursive S latch acquisition is not allowed. */
3467-
if (!block->lock.u_lock_try(false))
3464+
if (!block->lock.s_lock_try())
34683465
{
34693466
buf_block_buf_fix_dec(block);
34703467
return nullptr;
34713468
}
34723469

3473-
mtr_memo_push(mtr, block, MTR_MEMO_PAGE_SX_FIX);
3470+
mtr_memo_push(mtr, block, MTR_MEMO_PAGE_S_FIX);
34743471

34753472
#ifdef UNIV_DEBUG
34763473
if (!(++buf_dbg_counter % 5771)) buf_pool.validate();

storage/innobase/include/buf0buf.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,12 +232,12 @@ buf_page_optimistic_get(
232232
ib_uint64_t modify_clock,/*!< in: modify clock value */
233233
mtr_t* mtr); /*!< in: mini-transaction */
234234

235-
/** Try to U-latch a page.
235+
/** Try to S-latch a page.
236236
Suitable for using when holding the lock_sys latches (as it avoids deadlock).
237237
@param[in] page_id page identifier
238238
@param[in,out] mtr mini-transaction
239239
@return the block
240-
@retval nullptr if an U-latch cannot be granted immediately */
240+
@retval nullptr if an S-latch cannot be granted immediately */
241241
buf_block_t *buf_page_try_get(const page_id_t page_id, mtr_t *mtr);
242242

243243
/** Get read access to a compressed page (usually of type

storage/innobase/include/sux_lock.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ this program; if not, write to the Free Software Foundation, Inc.,
2121
#include "my_atomic_wrapper.h"
2222
#include "os0thread.h"
2323
#ifdef UNIV_DEBUG
24-
# include <set>
24+
# include <unordered_set>
2525
#endif
2626

2727
/** A "fat" rw-lock that supports
@@ -48,7 +48,7 @@ class sux_lock final
4848
/** Protects readers */
4949
mutable srw_mutex readers_lock;
5050
/** Threads that hold the lock in shared mode */
51-
std::atomic<std::set<os_thread_id_t>*> readers;
51+
std::atomic<std::unordered_multiset<os_thread_id_t>*> readers;
5252
#endif
5353

5454
/** The multiplier in recursive for X locks */
@@ -130,14 +130,15 @@ class sux_lock final
130130
/** Register the current thread as a holder of a shared lock */
131131
void s_lock_register()
132132
{
133+
const os_thread_id_t id= os_thread_get_curr_id();
133134
readers_lock.wr_lock();
134135
auto r= readers.load(std::memory_order_relaxed);
135136
if (!r)
136137
{
137-
r= new std::set<os_thread_id_t>();
138+
r= new std::unordered_multiset<os_thread_id_t>();
138139
readers.store(r, std::memory_order_relaxed);
139140
}
140-
ut_ad(r->emplace(os_thread_get_curr_id()).second);
141+
r->emplace(id);
141142
readers_lock.wr_unlock();
142143
}
143144
#endif
@@ -218,10 +219,13 @@ class sux_lock final
218219
void s_unlock()
219220
{
220221
#ifdef UNIV_DEBUG
222+
const os_thread_id_t id= os_thread_get_curr_id();
221223
auto r= readers.load(std::memory_order_relaxed);
222224
ut_ad(r);
223225
readers_lock.wr_lock();
224-
ut_ad(r->erase(os_thread_get_curr_id()) == 1);
226+
auto i= r->find(id);
227+
ut_ad(i != r->end());
228+
r->erase(i);
225229
readers_lock.wr_unlock();
226230
#endif
227231
lock.rd_unlock();

0 commit comments

Comments
 (0)