maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Sanja,

JFYI: while reporting this bug I came up with two questions that I couldn't
answer quickly. Probably they make sense, if not just ignore them:

1. This code was introduced with https://bugs.launchpad.net/maria/+bug/782223,
   revision: wlad@xxxxxxxxxxxxxxxx-20110514163720-rmico2qegtptjguk
  <quot>
  The callstack leading of "free" containing critical section is:
    mysqld!free
    my_no_flags_free
    Query_cache::free_cache
    Query_cache::resize
    fix_query_cache_size
    set_var::update
    sql_set_variables
    mysql_execute_command
    mysql_parse
  <quot>

  It means that this code was supposed to be executed exactly by
  Query_cache::resize(), where you disable it.

  Question: if this code is not supposed to be executed by ::resize(), is it
  needed at all?

2. In Query_cache::resize() I can see block-level locks are acquired with the
   following comment:
   <quot>
   Wait for all readers and writers to exit. When the list of all queries
   is iterated over with a block level lock, we are done.
   </quot>

   Isn't it needed in ::free_cache() also?

Regards,
Sergey

On Sat, Feb 08, 2014 at 01:25:11PM +0000, sanja@xxxxxxxxxxxx wrote:
> At file:///home/bell/maria/bzr/work-maria-5.5-MDEV-5607/
> 
> ------------------------------------------------------------
> revno: 4056
> revision-id: sanja@xxxxxxxxxxxx-20140208132448-qdkkpdncnya0tr6j
> parent: elenst@xxxxxxxxxxxxxx-20140205102537-7ern5gfca6a6ojg3
> committer: sanja@xxxxxxxxxxxx
> branch nick: work-maria-5.5-MDEV-5607
> timestamp: Sat 2014-02-08 15:24:48 +0200
> message:
>   MDEV-5607: Query cache destroys uninitialized rwlock
>   
>   - Resize destroyed rw_lock twice, now it is prevented.
>   - Trash destroyed rw lock.

> === modified file 'include/my_valgrind.h'
> --- a/include/my_valgrind.h	2012-03-22 11:31:09 +0000
> +++ b/include/my_valgrind.h	2014-02-08 13:24:48 +0000
> @@ -13,7 +13,8 @@
>     along with this program; if not, write to the Free Software
>     Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
>  
> -
> +#ifndef my_valgrind_h
> +#define my_valgrind_h
>  /* Some defines to make it easier to use valgrind */
>  #include <m_string.h>           /* bfill */
>  
> @@ -45,3 +46,4 @@
>  #define TRASH_FREE(A,B) TRASH_FILL(A,B,0x8F)
>  #define TRASH(A,B) TRASH_FREE(A,B)
>  
> +#endif //my_valgrind_h
> 
> === modified file 'include/mysql/psi/mysql_thread.h'
> --- a/include/mysql/psi/mysql_thread.h	2013-01-11 00:03:43 +0000
> +++ b/include/mysql/psi/mysql_thread.h	2014-02-08 13:24:48 +0000
> @@ -16,6 +16,8 @@
>  #ifndef MYSQL_THREAD_H
>  #define MYSQL_THREAD_H
>  
> +#include <my_valgrind.h>
> +
>  /**
>    @file mysql/psi/mysql_thread.h
>    Instrumentation helpers for mysys threads, mutexes,
> @@ -714,6 +716,7 @@ static inline int inline_mysql_prlock_in
>  static inline int inline_mysql_rwlock_destroy(
>    mysql_rwlock_t *that)
>  {
> +  int res;
>  #ifdef HAVE_PSI_INTERFACE
>    if (likely(PSI_server && that->m_psi))
>    {
> @@ -721,7 +724,9 @@ static inline int inline_mysql_rwlock_de
>      that->m_psi= NULL;
>    }
>  #endif
> -  return rwlock_destroy(&that->m_rwlock);
> +  res= rwlock_destroy(&that->m_rwlock);
> +  TRASH(that, sizeof(*that));
> +  return res;
>  }
>  
>  #ifndef DISABLE_MYSQL_PRLOCK_H
> 
> === modified file 'mysql-test/r/query_cache.result'
> --- a/mysql-test/r/query_cache.result	2013-10-16 13:07:25 +0000
> +++ b/mysql-test/r/query_cache.result	2014-02-08 13:24:48 +0000
> @@ -2077,6 +2077,21 @@ drop procedure p1;
>  drop table t1;
>  set GLOBAL query_cache_size=1355776;
>  SET GLOBAL userstat=default;
> +#
> +#MDEV-5607 Query cache destroys uninitialized rwlock
> +#
> +SET @global_query_cache_size = @@global.query_cache_size;
> +SET @global_query_cache_type = @@global.query_cache_type;
> +SET GLOBAL query_cache_type = default;
> +SET GLOBAL query_cache_size = default;
> +SET GLOBAL query_cache_type = ON;
> +SET GLOBAL query_cache_size = 131072;
> +CREATE TABLE t1(a INT);
> +SELECT * FROM t1;
> +a
> +SET GLOBAL query_cache_size = @global_query_cache_size;
> +SET GLOBAL query_cache_type = @global_query_cache_type;
> +DROP TABLE t1;
>  End of 5.5 tests
>  restore defaults
>  SET GLOBAL query_cache_type= default;
> 
> === modified file 'mysql-test/t/query_cache.test'
> --- a/mysql-test/t/query_cache.test	2013-10-16 13:07:25 +0000
> +++ b/mysql-test/t/query_cache.test	2014-02-08 13:24:48 +0000
> @@ -1708,6 +1708,23 @@ drop table t1;
>  set GLOBAL query_cache_size=1355776;
>  SET GLOBAL userstat=default;
>  
> +--echo #
> +--echo #MDEV-5607 Query cache destroys uninitialized rwlock
> +--echo #
> +SET @global_query_cache_size = @@global.query_cache_size;
> +SET @global_query_cache_type = @@global.query_cache_type;
> +SET GLOBAL query_cache_type = default;
> +SET GLOBAL query_cache_size = default;
> +SET GLOBAL query_cache_type = ON;
> +SET GLOBAL query_cache_size = 131072;
> +
> +CREATE TABLE t1(a INT);
> +SELECT * FROM t1;
> +
> +SET GLOBAL query_cache_size = @global_query_cache_size;
> +SET GLOBAL query_cache_type = @global_query_cache_type;
> +DROP TABLE t1;
> +
>  --echo End of 5.5 tests
>  
>  --echo restore defaults
> 
> === modified file 'sql/sql_cache.cc'
> --- a/sql/sql_cache.cc	2013-11-19 12:16:25 +0000
> +++ b/sql/sql_cache.cc	2014-02-08 13:24:48 +0000
> @@ -1326,6 +1326,7 @@ ulong Query_cache::resize(ulong query_ca
>        query->unlock_n_destroy();
>        block= block->next;
>      } while (block != queries_blocks);
> +    queries_blocks= NULL; // avoid second destroying by free_cache
>    }
>    free_cache();
>  
> 
> === modified file 'storage/perfschema/ha_perfschema.cc'
> --- a/storage/perfschema/ha_perfschema.cc	2012-03-27 23:04:46 +0000
> +++ b/storage/perfschema/ha_perfschema.cc	2014-02-08 13:24:48 +0000
> @@ -19,9 +19,9 @@
>  */
>  
>  #include "my_global.h"
> +#include "sql_plugin.h"
>  #include "my_pthread.h"
>  #include "my_atomic.h"
> -#include "sql_plugin.h"
>  #include "mysql/plugin.h"
>  #include "ha_perfschema.h"
>  #include "pfs_engine_table.h"
> 

> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits