MySQL Bugs: #49334: add_to_status() race conditions

Bug #49334	add_to_status() race conditions
Submitted:	2 Dec 2009 10:19	Modified:	28 Jun 2010 9:13
Reporter:	Sergei Golubchik	Email Updates:
Status:	Won't fix	Impact on me:	None
Category:	MySQL Server	Severity:	S3 (Non-critical)
Version:	5.1	OS:	Any
Assigned to:	Dmitry Shulga	CPU Architecture:	Any

Description:
add_to_status() that is used to add SHOW SESSION STATUS values to SHOW GLOBAL STATUS is not always protected by a mutex, and thus prone to race conditions.

How to repeat:
add

  safe_mutex_assert_owner(&LOCK_status);

to add_to_status(), start mysqld.
The test suite won't even run the bootstrap step

Thank you for the report.

Verified as described.

This is what happens during bootstrap stage with recent 5.1.42-bzr:

openxs@suse:/home2/openxs/bzr/mysql-5.1-work> tail -100 mysql-test/var/log/bootstrap.log
/home2/openxs/bzr/mysql-5.1-work/sql/mysqld --no-defaults --bootstrap --basedir=/home2/openxs/bzr/mysql-5.1-work --datadir=/home2/openxs/bzr/mysql-5.1-work/mysql-test/var/install.db --loose-skip-innodb --loose-skip-falcon --loose-skip-ndbcluster --tmpdir=/home2/openxs/bzr/mysql-5.1-work/mysql-test/var/tmp/ --core-file --language=/home2/openxs/bzr/mysql-5.1-work/sql/share/english --character-sets-dir=/home2/openxs/bzr/mysql-5.1-work/sql/share/charsets
100309 18:13:16 [Note] Plugin 'FEDERATED' is disabled.
100309 18:13:16 [Note] Plugin 'InnoDB' is disabled.
100309 18:13:16 [Note] Plugin 'ndbcluster' is disabled.
100309 18:13:16 [Warning] /home2/openxs/bzr/mysql-5.1-work/sql/mysqld: unknown option '--loose-skip-falcon'
mysqld: sql_class.cc:1025: void add_to_status(STATUS_VAR*, STATUS_VAR*): Assertion `(&LOCK_status)->count > 0 && pthread_equal(pthread_self(), (&LOCK_status)->thread)' failed.
100309 18:13:16 - mysqld got signal 6 ;
This could be because you hit a bug. It is also possible that this binary
or one of the libraries it was linked against is corrupt, improperly built,
or misconfigured. This error can also be caused by malfunctioning hardware.
We will try our best to scrape up some info that will hopefully help diagnose
the problem, but since we have already crashed, something is definitely wrong
and this may fail.

key_buffer_size=8384512
read_buffer_size=131072
max_used_connections=0
max_threads=151
threads_connected=0
It is possible that mysqld could use up to
key_buffer_size + (read_buffer_size + sort_buffer_size)*max_threads = 337727 K
bytes of memory
Hope that's ok; if not, decrease some variables in the equation.

thd: 0x8a2c628
Attempting backtrace. You can use the following information to find out
where mysqld died. If you see no messages after this, something went
terribly wrong...
stack_bottom = 0xbfffe59c thread_stack 0x30000
/home2/openxs/bzr/mysql-5.1-work/sql/mysqld(my_print_stacktrace+0x29)[0x86df774]
/home2/openxs/bzr/mysql-5.1-work/sql/mysqld(handle_segfault+0x253)[0x8292a27]
[0xffffe420]
/lib/tls/libc.so.6(abort+0x1a5)[0x400e6b75]
/lib/tls/libc.so.6(__assert_fail+0x103)[0x400de903]
/home2/openxs/bzr/mysql-5.1-work/sql/mysqld(_Z13add_to_statusP17system_status_varS0_+0x65)[0x82794db]
/home2/openxs/bzr/mysql-5.1-work/sql/mysqld(_ZN3THDD0Ev+0xf1)[0x82791ab]
/home2/openxs/bzr/mysql-5.1-work/sql/mysqld(_Z19mysql_rm_tmp_tablesv+0x443)[0x82fad8d]
/home2/openxs/bzr/mysql-5.1-work/sql/mysqld(main+0x293)[0x8295eb1]
/lib/tls/libc.so.6(__libc_start_main+0xd0)[0x400d2e80]
/home2/openxs/bzr/mysql-5.1-work/sql/mysqld[0x81ae6b1]
Trying to get some variables.
Some pointers may be invalid and cause the dump to abort...
thd->query at (nil) is an invalid pointer
thd->thread_id=0
thd->killed=NOT_KILLED
The manual page at http://dev.mysql.com/doc/mysql/en/crashing.html contains
information that should help you find out what is causing the crash.
Writing a core file

If this is to be made thread safe, please provide an option to use atomic add rather than holding a mutex.

In cases when add_to_status() is not protected by LOCK_status it is protected by LOCK_thread_count. 
Dmitry, please add the respective assert and close the bug.

add_to_status() adds two arbitrary STATUS_VAR objects. Adding an assert to it would reduce its applicability.
The code was manually checked and proof-read to ensure that either LOCK_status or LOCK_thread_count or (sometimes) both are held while adding session statistics to the global statistics.
No further action is considered necessary at this point (in 5.1) (the code needs to be rewritten to address this and other issues.
Closing the bug as "Won't fix" as no real action will be done in 5.1.

What's the point of having "either LOCK_status or LOCK_thread_count" held ?

If one thread took LOCK_status and another took LOCK_thread_count - they can execute the critical section in parallel.

Which, precisely, means that it is not protected.