From 97c7996e89978da3403848c971f686b6cd3b89f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 8 Feb 2013 09:22:46 +0200 Subject: [PATCH 1/2] Bug#16292043 RACE CONDITION IN SRV_EXPORT_INNODB_STATUS() WHEN ACCESSING PURGE_SYS->VIEW srv_export_innodb_status(): Read the purge_sys fields while holding purge_sys->latch. Approved by Sunny Bains --- storage/innobase/srv/srv0srv.c | 39 ++++++++++++++++---------- storage/innodb_plugin/srv/srv0srv.c | 43 ++++++++++++++++++----------- 2 files changed, 52 insertions(+), 30 deletions(-) diff --git a/storage/innobase/srv/srv0srv.c b/storage/innobase/srv/srv0srv.c index 3240b7515f8..e5619abbe45 100644 --- a/storage/innobase/srv/srv0srv.c +++ b/storage/innobase/srv/srv0srv.c @@ -1922,21 +1922,32 @@ srv_export_innodb_status(void) export_vars.innodb_rows_deleted = srv_n_rows_deleted; #ifdef UNIV_DEBUG - if (ut_dulint_cmp(trx_sys->max_trx_id, purge_sys->done_trx_no) < 0) { - export_vars.innodb_purge_trx_id_age = 0; - } else { - export_vars.innodb_purge_trx_id_age = - ut_dulint_minus(trx_sys->max_trx_id, purge_sys->done_trx_no); - } + { + dulint done_trx_no; + dulint up_limit_id; - if (!purge_sys->view - || ut_dulint_cmp(trx_sys->max_trx_id, - purge_sys->view->up_limit_id) < 0) { - export_vars.innodb_purge_view_trx_id_age = 0; - } else { - export_vars.innodb_purge_view_trx_id_age = - ut_dulint_minus(trx_sys->max_trx_id, - purge_sys->view->up_limit_id); + rw_lock_s_lock(&purge_sys->latch); + done_trx_no = purge_sys->done_trx_no; + up_limit_id = purge_sys->view + ? purge_sys->view->up_limit_id + : ut_dulint_zero; + rw_lock_s_unlock(&purge_sys->latch); + + if (ut_dulint_cmp(trx_sys->max_trx_id, done_trx_no) < 0) { + export_vars.innodb_purge_trx_id_age = 0; + } else { + export_vars.innodb_purge_trx_id_age = ut_dulint_minus( + trx_sys->max_trx_id, done_trx_no); + } + + if (ut_dulint_is_zero(up_limit_id) + || ut_dulint_cmp(trx_sys->max_trx_id, up_limit_id) < 0) { + export_vars.innodb_purge_view_trx_id_age = 0; + } else { + export_vars.innodb_purge_view_trx_id_age = + ut_dulint_minus(trx_sys->max_trx_id, + up_limit_id); + } } #endif /* UNIV_DEBUG */ diff --git a/storage/innodb_plugin/srv/srv0srv.c b/storage/innodb_plugin/srv/srv0srv.c index 4ddd0a4603d..4e626003f4f 100644 --- a/storage/innodb_plugin/srv/srv0srv.c +++ b/storage/innodb_plugin/srv/srv0srv.c @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 1995, 2010, Innobase Oy. All Rights Reserved. +Copyright (c) 1995, 2013, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2008, 2009 Google Inc. Copyright (c) 2009, Percona Inc. @@ -257,7 +257,7 @@ UNIV_INTERN ulint srv_data_read = 0; /* Internal setting for "innodb_stats_method". Decides how InnoDB treats NULL value when collecting statistics. By default, it is set to SRV_STATS_NULLS_EQUAL(0), ie. all NULL value are treated equal */ -ulong srv_innodb_stats_method = SRV_STATS_NULLS_EQUAL; +UNIV_INTERN ulong srv_innodb_stats_method = SRV_STATS_NULLS_EQUAL; /* here we count the amount of data written in total (in bytes) */ UNIV_INTERN ulint srv_data_written = 0; @@ -1978,21 +1978,32 @@ srv_export_innodb_status(void) export_vars.innodb_rows_deleted = srv_n_rows_deleted; #ifdef UNIV_DEBUG - if (ut_dulint_cmp(trx_sys->max_trx_id, purge_sys->done_trx_no) < 0) { - export_vars.innodb_purge_trx_id_age = 0; - } else { - export_vars.innodb_purge_trx_id_age = - ut_dulint_minus(trx_sys->max_trx_id, purge_sys->done_trx_no); - } + { + dulint done_trx_no; + dulint up_limit_id; - if (!purge_sys->view - || ut_dulint_cmp(trx_sys->max_trx_id, - purge_sys->view->up_limit_id) < 0) { - export_vars.innodb_purge_view_trx_id_age = 0; - } else { - export_vars.innodb_purge_view_trx_id_age = - ut_dulint_minus(trx_sys->max_trx_id, - purge_sys->view->up_limit_id); + rw_lock_s_lock(&purge_sys->latch); + done_trx_no = purge_sys->done_trx_no; + up_limit_id = purge_sys->view + ? purge_sys->view->up_limit_id + : ut_dulint_zero; + rw_lock_s_unlock(&purge_sys->latch); + + if (ut_dulint_cmp(trx_sys->max_trx_id, done_trx_no) < 0) { + export_vars.innodb_purge_trx_id_age = 0; + } else { + export_vars.innodb_purge_trx_id_age = ut_dulint_minus( + trx_sys->max_trx_id, done_trx_no); + } + + if (ut_dulint_is_zero(up_limit_id) + || ut_dulint_cmp(trx_sys->max_trx_id, up_limit_id) < 0) { + export_vars.innodb_purge_view_trx_id_age = 0; + } else { + export_vars.innodb_purge_view_trx_id_age = + ut_dulint_minus(trx_sys->max_trx_id, + up_limit_id); + } } #endif /* UNIV_DEBUG */ From 5620418c51f89c50796c03ed1d471abcecb704c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 8 Feb 2013 09:23:12 +0200 Subject: [PATCH 2/2] Add missing linkage specifiers, so that ha_innodb_plugin.so will not export internal symbols. --- storage/innodb_plugin/handler/ha_innodb.cc | 1 + storage/innodb_plugin/trx/trx0sys.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/storage/innodb_plugin/handler/ha_innodb.cc b/storage/innodb_plugin/handler/ha_innodb.cc index 229e7fe71e5..e0339b727ea 100644 --- a/storage/innodb_plugin/handler/ha_innodb.cc +++ b/storage/innodb_plugin/handler/ha_innodb.cc @@ -1067,6 +1067,7 @@ mysqld.cc. We do a dirty read because for one there is no synchronization object and secondly there is little harm in doing so even if we get a torn read. @return value of lower_case_table_names */ +static ulint innobase_get_lower_case_table_names(void) /*=====================================*/ diff --git a/storage/innodb_plugin/trx/trx0sys.c b/storage/innodb_plugin/trx/trx0sys.c index daa65bfcef0..6b44db14af7 100644 --- a/storage/innodb_plugin/trx/trx0sys.c +++ b/storage/innodb_plugin/trx/trx0sys.c @@ -129,7 +129,7 @@ static const ulint FILE_FORMAT_NAME_N #ifdef UNIV_DEBUG /* Flag to control TRX_RSEG_N_SLOTS behavior debugging. */ -uint trx_rseg_n_slots_debug = 0; +UNIV_INTERN uint trx_rseg_n_slots_debug = 0; #endif #ifndef UNIV_HOTBACKUP