Cleanups:

- Don't use SAFEMALLOC on valgrind builds (slows things down)
- Added back lost option from 5.3: debug-mutex-deadlock-detector
- Flush pages before taking lock mutex (speeds up closing of Aria tables).

BUILD/SETUP.sh:
  - Don't use SAFEMALLOC on valgrind builds (slows things down)
sql/lock.cc:
  Make default argument explicit (improves readability)
sql/mysqld.cc:
  Removed compiler warnings
  Sorted debug options alphabetically
  Added back lost option from 5.3: debug-mutex-deadlock-detector
storage/maria/ma_close.c:
  Flush pages before taking lock mutex (speeds up closing of Aria tables).
storage/maria/ma_open.c:
  More DBUG_PRINT
storage/maria/maria_def.h:
  Better DBUG_PRINT
storage/maria/trnman.c:
  Better DBUG_PRINT

BUILD/SETUP.sh

@@ -148,6 +148,7 @@ fi
 # Override -DFORCE_INIT_OF_VARS from debug_cflags. It enables the macro
 # LINT_INIT(), which is only useful for silencing spurious warnings
 # of static analysis tools. We want LINT_INIT() to be a no-op in Valgrind.
+valgrind_flags="-DHAVE_valgrind -USAFEMALLOC"
 valgrind_flags="$valgrind_flags -UFORCE_INIT_OF_VARS -Wno-uninitialized"
 valgrind_flags="$valgrind_flags -DMYSQL_SERVER_SUFFIX=-valgrind-max"
 valgrind_configs="--with-valgrind"

sql/lock.cc

@@ -399,7 +399,7 @@ void mysql_unlock_some_tables(THD *thd, TABLE **table,uint count)
 {
   MYSQL_LOCK *sql_lock;
   if ((sql_lock= get_lock_data(thd, table, count, GET_LOCK_UNLOCK)))
-    mysql_unlock_tables(thd, sql_lock);
+    mysql_unlock_tables(thd, sql_lock, 1);
 }
 
 

sql/mysqld.cc

@@ -5424,8 +5424,10 @@ void handle_connections_sockets()
   uint error_count=0;
   THD *thd;
   struct sockaddr_storage cAddr;
-  int ip_flags=0,socket_flags=0,flags=0,retval;
-  int extra_ip_flags=0;
+  int ip_flags __attribute__((unused))=0;
+  int socket_flags __attribute__((unused))= 0;
+  int extra_ip_flags __attribute__((unused))=0;
+  int flags=0,retval;
   st_vio *vio_tmp;
 #ifdef HAVE_POLL
   int socket_count= 0;
@@ -6014,17 +6016,6 @@ struct my_option my_long_options[]=
   {"help", '?', "Display this help and exit.", 
    &opt_help, &opt_help, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0,
    0, 0},
-#ifdef DBUG_OFF
-  {"debug", '#', "Built in DBUG debugger. Disabled in this build.",
-   &current_dbug_option, &current_dbug_option, 0, GET_STR, OPT_ARG,
-   0, 0, 0, 0, 0, 0},
-#endif
-#ifdef HAVE_REPLICATION
-  {"debug-abort-slave-event-count", 0,
-   "Option used by mysql-test for debugging and testing of replication.",
-   &abort_slave_event_count,  &abort_slave_event_count,
-   0, GET_INT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
-#endif /* HAVE_REPLICATION */
   {"allow-suspicious-udfs", 0,
    "Allows use of UDFs consisting of only one symbol xxx() "
    "without corresponding xxx_init() or xxx_deinit(). That also means "
@@ -6093,30 +6084,86 @@ struct my_option my_long_options[]=
   /* default-storage-engine should have "MyISAM" as def_value. Instead
      of initializing it here it is done in init_common_variables() due
      to a compiler bug in Sun Studio compiler. */
+#ifdef DBUG_OFF
+  {"debug", '#', "Built in DBUG debugger. Disabled in this build.",
+   &current_dbug_option, &current_dbug_option, 0, GET_STR, OPT_ARG,
+   0, 0, 0, 0, 0, 0},
+#endif
+#ifdef HAVE_REPLICATION
+  {"debug-abort-slave-event-count", 0,
+   "Option used by mysql-test for debugging and testing of replication.",
+   &abort_slave_event_count,  &abort_slave_event_count,
+   0, GET_INT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
+#endif /* HAVE_REPLICATION */
+#ifndef DBUG_OFF
+  {"debug-assert-on-error", 0,
+   "Do an assert in various functions if we get a fatal error",
+   &my_assert_on_error, &my_assert_on_error,
+   0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
+  {"debug-assert-if-crashed-table", 0,
+   "Do an assert in handler::print_error() if we get a crashed table",
+   &debug_assert_if_crashed_table, &debug_assert_if_crashed_table,
+   0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
+#endif
+#ifdef HAVE_REPLICATION
+  {"debug-disconnect-slave-event-count", 0,
+   "Option used by mysql-test for debugging and testing of replication.",
+   &disconnect_slave_event_count, &disconnect_slave_event_count,
+   0, GET_INT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
+#endif /* HAVE_REPLICATION */
+  {"debug-exit-info", 'T', "Used for debugging. Use at your own risk.",
+   0, 0, 0, GET_LONG, OPT_ARG, 0, 0, 0, 0, 0, 0},
+  {"debug-gdb", 0,
+   "Set up signals usable for debugging.",
+   &opt_debugging, &opt_debugging,
+   0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
+#ifdef HAVE_REPLICATION
+  {"debug-max-binlog-dump-events", 0,
+   "Option used by mysql-test for debugging and testing of replication.",
+   &max_binlog_dump_events, &max_binlog_dump_events, 0,
+   GET_INT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
+#endif /* HAVE_REPLICATION */
+#ifdef SAFE_MUTEX
+  {"debug-mutex-deadlock-detector", 0,
+   "Enable checking of wrong mutex usage.",
+   &safe_mutex_deadlock_detector,
+   &safe_mutex_deadlock_detector,
+   0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0},
+#endif
+  {"debug-no-sync", 0,
+   "Disables system sync calls. Only for running tests or debugging!",
+   &my_disable_sync, &my_disable_sync, 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0},
+#ifdef HAVE_REPLICATION
+  {"debug-sporadic-binlog-dump-fail", 0,
+   "Option used by mysql-test for debugging and testing of replication.",
+   &opt_sporadic_binlog_dump_fail,
+   &opt_sporadic_binlog_dump_fail, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0,
+   0},
+#endif /* HAVE_REPLICATION */
   {"default-storage-engine", 0, "The default storage engine for new tables",
    &default_storage_engine, 0, 0, GET_STR, REQUIRED_ARG,
    0, 0, 0, 0, 0, 0 },
   {"default-time-zone", 0, "Set the default time zone.",
    &default_tz_name, &default_tz_name,
    0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0 },
+#if defined(ENABLED_DEBUG_SYNC)
+  {"debug-sync-timeout", OPT_DEBUG_SYNC_TIMEOUT,
+   "Enable the debug sync facility "
+   "and optionally specify a default wait timeout in seconds. "
+   "A zero value keeps the facility disabled.",
+   &opt_debug_sync_timeout, 0,
+   0, GET_UINT, OPT_ARG, 0, 0, UINT_MAX, 0, 0, 0},
+#endif /* defined(ENABLED_DEBUG_SYNC) */
 #ifdef HAVE_OPENSSL
   {"des-key-file", 0,
    "Load keys for des_encrypt() and des_encrypt from given file.",
    &des_key_file, &des_key_file, 0, GET_STR, REQUIRED_ARG,
    0, 0, 0, 0, 0, 0},
 #endif /* HAVE_OPENSSL */
-#ifdef HAVE_REPLICATION
-  {"debug-disconnect-slave-event-count", 0,
-   "Option used by mysql-test for debugging and testing of replication.",
-   &disconnect_slave_event_count, &disconnect_slave_event_count,
-   0, GET_INT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
-#endif /* HAVE_REPLICATION */
 #ifdef HAVE_STACKTRACE
   {"stack-trace", 0 , "Print a symbolic stack trace on failure",
    &opt_stack_trace, &opt_stack_trace, 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0},
 #endif /* HAVE_STACKTRACE */
-  {"debug-exit-info", 'T', "Used for debugging. Use at your own risk.", 0, 0, 0,
-   GET_LONG, OPT_ARG, 0, 0, 0, 0, 0, 0},
   {"external-locking", 0, "Use system (external) locking (disabled by "
    "default).  With this option enabled you can run myisamchk to test "
    "(not repair) tables while the MySQL server is running. Disable with "
@@ -6128,10 +6175,6 @@ struct my_option my_long_options[]=
    "Set up signals usable for debugging. Deprecated, use --debug-gdb instead.",
    &opt_debugging, &opt_debugging,
    0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
-  {"debug-gdb", 0,
-   "Set up signals usable for debugging.",
-   &opt_debugging, &opt_debugging,
-   0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
 #ifdef HAVE_LARGE_PAGE_OPTION
   {"super-large-pages", 0, "Enable support for super large pages.",
    &opt_super_large_pages, &opt_super_large_pages, 0,
@@ -6222,10 +6265,6 @@ struct my_option my_long_options[]=
   {"init-rpl-role", 0, "Set the replication role.",
    &rpl_status, &rpl_status, &rpl_role_typelib,
    GET_ENUM, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
-  {"debug-max-binlog-dump-events", 0,
-   "Option used by mysql-test for debugging and testing of replication.",
-   &max_binlog_dump_events, &max_binlog_dump_events, 0,
-   GET_INT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
 #endif /* HAVE_REPLICATION */
   {"memlock", 0, "Lock mysqld in memory.", &locked_in_memory,
    &locked_in_memory, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
@@ -6331,13 +6370,6 @@ struct my_option my_long_options[]=
    "(Default: 15000).", &slow_start_timeout, &slow_start_timeout, 0,
    GET_ULONG, REQUIRED_ARG, 15000, 0, 0, 0, 0, 0},
 #endif
-#ifdef HAVE_REPLICATION
-  {"debug-sporadic-binlog-dump-fail", 0,
-   "Option used by mysql-test for debugging and testing of replication.",
-   &opt_sporadic_binlog_dump_fail,
-   &opt_sporadic_binlog_dump_fail, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0,
-   0},
-#endif /* HAVE_REPLICATION */
 #ifdef HAVE_OPENSSL
   {"ssl", 0,
    "Enable SSL for connection (automatically enabled with other flags).",
@@ -6359,9 +6391,6 @@ struct my_option my_long_options[]=
      files.
    */
    IF_VALGRIND(0,IF_WIN(0,1)), 0, 0, 0, 0, 0},
-  {"debug-no-sync", 0,
-   "Disables system sync calls. Only for running tests or debugging!",
-   &my_disable_sync, &my_disable_sync, 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0},
   {"sysdate-is-now", 0,
    "Non-default option to alias SYSDATE() to NOW() to make it safe-replicable. "
    "Since 5.0, SYSDATE() returns a `dynamic' value different for different "
@@ -6372,14 +6401,6 @@ struct my_option my_long_options[]=
    "Decision to use in heuristic recover process. Possible values are COMMIT "
    "or ROLLBACK.", &tc_heuristic_recover, &tc_heuristic_recover,
    &tc_heuristic_recover_typelib, GET_ENUM, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
-#if defined(ENABLED_DEBUG_SYNC)
-  {"debug-sync-timeout", OPT_DEBUG_SYNC_TIMEOUT,
-   "Enable the debug sync facility "
-   "and optionally specify a default wait timeout in seconds. "
-   "A zero value keeps the facility disabled.",
-   &opt_debug_sync_timeout, 0,
-   0, GET_UINT, OPT_ARG, 0, 0, UINT_MAX, 0, 0, 0},
-#endif /* defined(ENABLED_DEBUG_SYNC) */
   {"temp-pool", 0,
 #if (ENABLE_TEMP_POOL)
    "Using this option will cause most temporary files created to use a small "
@@ -6409,16 +6430,6 @@ struct my_option my_long_options[]=
   {"table_cache", 0, "Deprecated; use --table-open-cache instead.",
    &table_cache_size, &table_cache_size, 0, GET_ULONG,
    REQUIRED_ARG, TABLE_OPEN_CACHE_DEFAULT, 1, 512*1024L, 0, 1, 0},
-#ifndef DBUG_OFF
-  {"debug-assert-on-error", 0,
-   "Do an assert in various functions if we get a fatal error",
-   &my_assert_on_error, &my_assert_on_error,
-   0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
-  {"debug-assert-if-crashed-table", 0,
-   "Do an assert in handler::print_error() if we get a crashed table",
-   &debug_assert_if_crashed_table, &debug_assert_if_crashed_table,
-   0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
-#endif
   {0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}
 };
 

storage/maria/ma_close.c

@@ -36,6 +36,21 @@ int maria_close(register MARIA_HA *info)
   /* Check that we have unlocked key delete-links properly */
   DBUG_ASSERT(info->key_del_used == 0);
 
+  if (share->reopen == 1)
+  {
+    /*
+      If we are going to close the file, flush page cache without
+      a global mutex
+    */
+    if (flush_pagecache_blocks(share->pagecache, &share->kfile,
+                               ((share->temporary || share->deleting) ?
+                                FLUSH_IGNORE_CHANGED :
+                                FLUSH_RELEASE)))
+      error= my_errno;
+  }
+
+
+  /* Ensure no one can open this file while we are closing it */
   mysql_mutex_lock(&THR_LOCK_maria);
   if (info->lock_type == F_EXTRA_LCK)
     info->lock_type=F_UNLCK;			/* HA_EXTRA_NO_USER_CHANGE */
@@ -81,6 +96,10 @@ int maria_close(register MARIA_HA *info)
 
       if ((*share->once_end)(share))
         error= my_errno;
+      /*
+        Extra flush, just in case someone opened and closed the file
+        since the start of the function (very unlikely)
+      */
       if (flush_pagecache_blocks(share->pagecache, &share->kfile,
                                  ((share->temporary || share->deleting) ?
                                   FLUSH_IGNORE_CHANGED :

storage/maria/ma_open.c

@@ -860,7 +860,7 @@ MARIA_HA *maria_open(const char *name, int mode, uint open_flags)
       {
         /*
           Move history from hash to share. This is safe to do as we
-          don't have a lock on share->intern_lock.
+          know we are the only one that is using the share.
         */
         share->state_history=
           _ma_remove_not_visible_states(history->state_history, 0, 0);
@@ -1352,6 +1352,7 @@ uint _ma_state_info_write_sub(File file, MARIA_STATE_INFO *state, uint pWrite)
   uint	i, keys= (uint) state->header.keys;
   size_t res;
   DBUG_ENTER("_ma_state_info_write_sub");
+  DBUG_PRINT("info", ("Records: %lld", state->state.records));
 
   memcpy(ptr,&state->header,sizeof(state->header));
   ptr+=sizeof(state->header);
@@ -1423,6 +1424,8 @@ uint _ma_state_info_write_sub(File file, MARIA_STATE_INFO *state, uint pWrite)
 static uchar *_ma_state_info_read(uchar *ptr, MARIA_STATE_INFO *state)
 {
   uint i,keys,key_parts;
+  DBUG_ENTER("_ma_state_info_read");
+
   memcpy(&state->header,ptr, sizeof(state->header));
   ptr+= sizeof(state->header);
   keys= (uint) state->header.keys;
@@ -1471,7 +1474,9 @@ static uchar *_ma_state_info_read(uchar *ptr, MARIA_STATE_INFO *state)
     float8get(state->rec_per_key_part[i], ptr);		ptr+= 8;
     state->nulls_per_key_part[i]= mi_uint4korr(ptr);	ptr+= 4;
   }
-  return ptr;
+
+  DBUG_PRINT("info", ("Records: %lld", state->state.records));
+  DBUG_RETURN(ptr);
 }
 
 

storage/maria/trnman.c

@@ -413,7 +413,7 @@ my_bool trnman_end_trn(TRN *trn, my_bool commit)
   TRN *free_me= 0;
   LF_PINS *pins= trn->pins;
   DBUG_ENTER("trnman_end_trn");
-  DBUG_PRINT("enter", ("trn=0x%lx commit=%d", (ulong) trn, commit));
+  DBUG_PRINT("enter", ("trn: %p  commit: %d", trn, commit));
 
   /* if a rollback, all UNDO records should have been executed */
   DBUG_ASSERT(commit || trn->undo_lsn == 0);