MySQL Bugs: #102809: A deadlock

Bug #102809	A deadlock
Submitted:	4 Mar 2021 2:22	Modified:	9 Mar 2021 11:06
Reporter:	linfeng chen	Email Updates:
Status:	Not a Bug	Impact on me:	None
Category:	MySQL Server: InnoDB storage engine	Severity:	S3 (Non-critical)
Version:	5.7.21	OS:	Any
Assigned to:		CPU Architecture:	Any

Description:
I found a piece of code that produces a deadlock.
The code is a little too much, please read through patiently.
fil_reinit_space_header_for_table()
{
......
btr_search_s_lock_all();
buf_LRU_flush_or_remove_pages(id, BUF_REMOVE_ALL_NO_WRITE, 0);
btr_search_s_unlock_all();
......
}
buf_LRU_flush_or_remove_pages()
{
......
buf_LRU_drop_page_hash_for_tablespace(buf_pool, id);
......
}
buf_LRU_drop_page_hash_for_tablespace()
{
......
buf_LRU_drop_page_hash_batch(id, page_size, page_arr, num_entries);
......
}
buf_LRU_drop_page_hash_batch()
{
......
btr_search_drop_page_hash_when_freed(
			page_id_t(space_id, *arr), page_size);
......
}
btr_search_drop_page_hash_when_freed()
{
......
btr_search_drop_page_hash_index(block);
......
}
btr_search_drop_page_hash_index()
{
......
rw_lock_s_lock(latch);
......
rw_lock_s_unlock(latch);
......
rw_lock_x_lock(latch);//A deadlock will occur here
}

How to repeat:
1:Modify the code so that the adapting hash data is not cleared when the index is dropped.
if (ahi) {
		btr_search_drop_page_hash_when_freed(page_id, page_size);
	}

	if (ahi) {
		for (i = 0; i < FSP_EXTENT_SIZE; i++) {
			if (!xdes_mtr_get_bit(descr, XDES_FREE_BIT, i, mtr)) {

				/* Drop search system page hash index
				if the page is found in the pool and
				is hashed */

				btr_search_drop_page_hash_when_freed(
					page_id_t(space,
						  first_page_in_extent + i),
					page_size);
			}
		}
	}
2.Turn on adaptive hashing.
3.Create a table that can use adaptive hashing.
4.Do something to generate the adaptive hash data for the table.
5.truncate table_name;

Suggested fix:
remove btr_search_s_lock_all();
buf_LRU_flush_or_remove_pages  function will not find the adaptive hash data.
No need for protection.
If you want to prevent closing adaptive hashing,a new lock should be added to implement this. 
Please answer me, can i delete btr_search_s_lock_all();
I know the new version has been modified, but I can only use the old version.

Hi Mr. chen,

Thank you for your bug report.

First of all,  we are only accepting reports for the most recent releases. Hence, you should test and analyse the code in 5.7.34, which has more than 5.000 bugs fixed since the release that you are using.

Next, if you are posting a report about mutex or lock contention (or deadlock), with need a full analysis to the finest detail on how could two, or more, threads produce a deadlock. We do not see that analysis in your report. 

In short, without the above two conditions being satisfied, we can not progress with processing of this report.

We shall wait on your full feedback.

1.This issue still exists in version 5.7.34,The code has not changed.
2.It is not necessary for two or more threads to create a deadlock.A thread can also be created.For example, a thread adds lock S twice, unlocks lock S only once, and then adds lock X.I'm sure you didn't understand what I said.Let me explain it a little bit more carefully.
3.
fil_reinit_space_header_for_table()
{
......
btr_search_s_lock_all();//This function adds multiple s-locks
/*void
btr_search_s_lock_all()
{
	for (ulint i = 0; i < btr_ahi_parts; ++i) {
		rw_lock_s_lock(btr_search_latches[i]);
	}
}*/

buf_LRU_flush_or_remove_pages(id, BUF_REMOVE_ALL_NO_WRITE, 0);
btr_search_s_unlock_all();
......
}
buf_LRU_flush_or_remove_pages()
{
......
buf_LRU_drop_page_hash_for_tablespace(buf_pool, id);
......
}
buf_LRU_drop_page_hash_for_tablespace()
{
......
buf_LRU_drop_page_hash_batch(id, page_size, page_arr, num_entries);
......
}
buf_LRU_drop_page_hash_batch()
{
......
btr_search_drop_page_hash_when_freed(
			page_id_t(space_id, *arr), page_size);
......
}
btr_search_drop_page_hash_when_freed()
{
......
btr_search_drop_page_hash_index(block);
......
}
btr_search_drop_page_hash_index()
{
......
rw_lock_s_lock(latch);
......
rw_lock_s_unlock(latch);
......
rw_lock_x_lock(latch);//A deadlock will occur here,because he put an S-lock on the front
}

Hi Mr. chen,

Thank you for your feedback.

Thanks to your explanation we were able to conclude what is it that you are reporting.

A single thread can hold only one record lock on one record. So, if a thread holds S lock on a record and ask another S lock of the same type, then it will be ignored. If a thread holds S lock on a record and asks for X lock of the same type, then it is called lock escalation, so it will be granted, unless some other thread holds already X lock. Anyway, theory of transactional databases has defined this behaviour to the last detail and InnoDB storage engine is following it to the last letter.

Not a bug.

It is a bug.
I want you to read the code carefully.
I've actually tested it. Deadlocks will happen.
There's something you're mistaken.
if a thread holds S lock on a record and ask another S lock of the same type, then it will be ignored. This is wrong.
if a thread holds S lock on a record and ask another S lock of the same type, then it will not be ignored.
The code is as follows:
void
rw_lock_s_lock_func(
/*================*/
	rw_lock_t*	lock,	
	ulint		pass,				
	const char*	file_name,
	ulint		line)	{
	ut_ad(!rw_lock_own(lock, RW_LOCK_S)); 
	ut_ad(!rw_lock_own(lock, RW_LOCK_X));
	if (!rw_lock_s_lock_low(lock, pass, file_name, line)) {
		rw_lock_s_lock_spin(lock, pass, file_name, line);
	}
}
In fact, it's going to run down in the debug version.
A deadlock occurs in the release version.
I'll wait for your reply.

Hi,

What you describe is, indeed, the intended behaviour.

Based on the above analysis, we should not add this lock.

reasons: Based on the above analysis, we should not add this lock.