Merge mysql.com:/Users/davi/mysql/bugs/32395-5.1

into  mysql.com:/Users/davi/mysql/mysql-5.1-runtime

mysql-test/r/lock_multi.result

@@ -113,4 +113,34 @@ handler t1 open;
 ERROR HY000: Table storage engine for 't1' doesn't have this option
 --> client 1
 drop table t1;
+drop table if exists t1;
+create table t1 (i int);
+connection: default
+lock tables t1 write;
+connection: flush
+flush tables with read lock;;
+connection: default
+alter table t1 add column j int;
+connection: insert
+insert into t1 values (1,2);;
+connection: default
+unlock tables;
+connection: flush
+select * from t1;
+i	j
+unlock tables;
+select * from t1;
+i	j
+1	2
+drop table t1;
+drop table if exists t1;
+create table t1 (i int);
+connection: default
+lock tables t1 write;
+connection: flush
+flush tables with read lock;;
+connection: default
+flush tables;
+unlock tables;
+drop table t1;
 End of 5.1 tests

mysql-test/r/trigger_notembedded.result

@@ -448,4 +448,18 @@ DROP TABLE t1;
 DROP DATABASE mysqltest_db1;
 USE test;
 End of 5.0 tests.
+drop table if exists t1;
+create table t1 (i int);
+connection: default
+lock tables t1 write;
+connection: flush
+flush tables with read lock;;
+connection: default
+create trigger t1_bi before insert on t1 for each row begin end;
+unlock tables;
+connection: flush
+unlock tables;
+select * from t1;
+i
+drop table t1;
 End of 5.1 tests.

mysql-test/t/lock_multi.test

@@ -150,7 +150,7 @@ send SELECT user.Select_priv FROM user, db WHERE user.user = db.user LIMIT 1;
 connection locker;
 let $wait_condition=
   select count(*) = 1 from information_schema.processlist
-  where state = "Locked" and info = 
+  where state = "Waiting for table" and info = 
   "SELECT user.Select_priv FROM user, db WHERE user.user = db.user LIMIT 1";
 --source include/wait_condition.inc
 # Make test case independent from earlier grants.
@@ -343,4 +343,100 @@ handler t1 open;
 connection default;
 drop table t1;
 
+#
+# Bug#32395 Alter table under a impending global read lock causes a server crash
+#
+
+#
+# Test ALTER TABLE under LOCK TABLES and FLUSH TABLES WITH READ LOCK
+#
+
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
+create table t1 (i int);
+connect (flush,localhost,root,,test,,);
+connection default;
+--echo connection: default
+lock tables t1 write;
+connection flush;
+--echo connection: flush
+--send flush tables with read lock;
+connection default;
+--echo connection: default
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Flushing tables";
+--source include/wait_condition.inc
+alter table t1 add column j int;
+connect (insert,localhost,root,,test,,);
+connection insert;
+--echo connection: insert
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Flushing tables";
+--source include/wait_condition.inc
+--send insert into t1 values (1,2);
+--echo connection: default
+connection default;
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Waiting for release of readlock";
+--source include/wait_condition.inc
+unlock tables;
+connection flush;
+--echo connection: flush
+--reap
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Waiting for release of readlock";
+--source include/wait_condition.inc
+select * from t1;
+unlock tables;
+connection insert;
+--reap
+connection default;
+select * from t1;
+drop table t1;
+disconnect flush;
+disconnect insert;
+
+#
+# Test that FLUSH TABLES under LOCK TABLES protects write locked tables
+# from a impending FLUSH TABLES WITH READ LOCK
+#
+
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
+create table t1 (i int);
+connect (flush,localhost,root,,test,,);
+connection default;
+--echo connection: default
+lock tables t1 write;
+connection flush;
+--echo connection: flush
+--send flush tables with read lock;
+connection default;
+--echo connection: default
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Flushing tables";
+--source include/wait_condition.inc
+flush tables;
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Flushing tables";
+--source include/wait_condition.inc
+unlock tables;
+let $wait_condition=
+  select count(*) = 0 from information_schema.processlist
+  where state = "Flushing tables";
+--source include/wait_condition.inc
+connection flush;
+--reap
+connection default;
+disconnect flush;
+drop table t1;
+
 --echo End of 5.1 tests

mysql-test/t/trigger_notembedded.test

@@ -880,8 +880,9 @@ USE test;
 # Bug#23713 LOCK TABLES + CREATE TRIGGER + FLUSH TABLES WITH READ LOCK = deadlock
 #
 
-# Test temporarily disable due to Bug#32395
---disable_parsing
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
 create table t1 (i int);
 connect (flush,localhost,root,,test,,);
 connection default;
@@ -906,6 +907,5 @@ connection default;
 select * from t1;
 drop table t1;
 disconnect flush;
---enable_parsing
 
 --echo End of 5.1 tests.

sql/ha_ndbcluster.cc

@@ -577,7 +577,7 @@ int ha_ndbcluster::ndb_err(NdbTransaction *trans)
     bzero((char*) &table_list,sizeof(table_list));
     table_list.db= m_dbname;
     table_list.alias= table_list.table_name= m_tabname;
-    close_cached_tables(thd, 0, &table_list);
+    close_cached_tables(thd, &table_list, FALSE, FALSE, FALSE);
     break;
   }
   default:
@@ -8064,7 +8064,7 @@ int handle_trailing_share(NDB_SHARE *share)
   table_list.db= share->db;
   table_list.alias= table_list.table_name= share->table_name;
   safe_mutex_assert_owner(&LOCK_open);
-  close_cached_tables(thd, 0, &table_list, TRUE);
+  close_cached_tables(thd, &table_list, TRUE, FALSE, FALSE);
 
   pthread_mutex_lock(&ndbcluster_mutex);
   /* ndb_share reference temporary free */

sql/ha_ndbcluster_binlog.cc

@@ -900,7 +900,7 @@ int ndbcluster_setup_binlog_table_shares(THD *thd)
     {
       if (ndb_extra_logging)
         sql_print_information("NDB Binlog: ndb tables writable");
-      close_cached_tables((THD*) 0, 0, (TABLE_LIST*) 0, TRUE);
+      close_cached_tables(NULL, NULL, TRUE, FALSE, FALSE);
     }
     pthread_mutex_unlock(&LOCK_open);
     /* Signal injector thread that all is setup */
@@ -1700,7 +1700,7 @@ ndb_handle_schema_change(THD *thd, Ndb *ndb, NdbEventOperation *pOp,
       bzero((char*) &table_list,sizeof(table_list));
       table_list.db= (char *)dbname;
       table_list.alias= table_list.table_name= (char *)tabname;
-      close_cached_tables(thd, 0, &table_list, TRUE);
+      close_cached_tables(thd, &table_list, TRUE, FALSE, FALSE);
 
       if ((error= ndbcluster_binlog_open_table(thd, share,
                                                table_share, table, 1)))
@@ -1806,7 +1806,7 @@ ndb_handle_schema_change(THD *thd, Ndb *ndb, NdbEventOperation *pOp,
     bzero((char*) &table_list,sizeof(table_list));
     table_list.db= (char *)dbname;
     table_list.alias= table_list.table_name= (char *)tabname;
-    close_cached_tables(thd, 0, &table_list);
+    close_cached_tables(thd, &table_list, FALSE, FALSE, FALSE);
     /* ndb_share reference create free */
     DBUG_PRINT("NDB_SHARE", ("%s create free  use_count: %u",
                              share->key, share->use_count));
@@ -1925,7 +1925,7 @@ ndb_binlog_thread_handle_schema_event(THD *thd, Ndb *ndb,
             bzero((char*) &table_list,sizeof(table_list));
             table_list.db= schema->db;
             table_list.alias= table_list.table_name= schema->name;
-            close_cached_tables(thd, 0, &table_list, FALSE);
+            close_cached_tables(thd, &table_list, FALSE, FALSE, FALSE);
           }
           /* ndb_share reference temporary free */
           if (share)
@@ -2049,7 +2049,7 @@ ndb_binlog_thread_handle_schema_event(THD *thd, Ndb *ndb,
       pthread_mutex_unlock(&ndb_schema_share_mutex);
       /* end protect ndb_schema_share */
 
-      close_cached_tables((THD*) 0, 0, (TABLE_LIST*) 0, FALSE);
+      close_cached_tables(NULL, NULL, FALSE, FALSE, FALSE);
       // fall through
     case NDBEVENT::TE_ALTER:
       ndb_handle_schema_change(thd, ndb, pOp, tmp_share);
@@ -2206,7 +2206,7 @@ ndb_binlog_thread_handle_schema_event_post_epoch(THD *thd,
           bzero((char*) &table_list,sizeof(table_list));
           table_list.db= schema->db;
           table_list.alias= table_list.table_name= schema->name;
-          close_cached_tables(thd, 0, &table_list, FALSE);
+          close_cached_tables(thd, &table_list, FALSE, FALSE, FALSE);
         }
         if (schema_type != SOT_ALTER_TABLE)
           break;

sql/mysql_priv.h

@@ -1595,7 +1595,8 @@ TABLE *open_performance_schema_table(THD *thd, TABLE_LIST *one_table,
                                      Open_tables_state *backup);
 void close_performance_schema_table(THD *thd, Open_tables_state *backup);
 
-bool close_cached_tables(THD *thd, bool wait_for_refresh, TABLE_LIST *tables, bool have_lock = FALSE);
+bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
+                         bool wait_for_refresh, bool wait_for_placeholders);
 bool close_cached_connection_tables(THD *thd, bool wait_for_refresh,
                                     LEX_STRING *connect_string,
                                     bool have_lock = FALSE);

sql/set_var.cc

@@ -3720,7 +3720,7 @@ bool sys_var_opt_readonly::update(THD *thd, set_var *var)
     can cause to wait on a read lock, it's required for the client application
     to unlock everything, and acceptable for the server to wait on all locks.
   */
-  if (result= close_cached_tables(thd, true, NULL, false))
+  if (result= close_cached_tables(thd, NULL, FALSE, TRUE, TRUE))
     goto end_with_read_lock;
 
   if (result= make_global_read_lock_block_commit(thd))

sql/sql_base.cc

@@ -130,7 +130,7 @@ void table_cache_free(void)
   DBUG_ENTER("table_cache_free");
   if (table_def_inited)
   {
-    close_cached_tables((THD*) 0,0,(TABLE_LIST*) 0);
+    close_cached_tables(NULL, NULL, FALSE, FALSE, FALSE);
     if (!open_cache.records)			// Safety first
       hash_free(&open_cache);
   }
@@ -908,16 +908,24 @@ void free_io_cache(TABLE *table)
 /*
   Close all tables which aren't in use by any thread
 
-  THD can be NULL, but then if_wait_for_refresh must be FALSE
-  and tables must be NULL.
+  @param thd Thread context
+  @param tables List of tables to remove from the cache
+  @param have_lock If LOCK_open is locked
+  @param wait_for_refresh Wait for a impending flush
+  @param wait_for_placeholders Wait for tables being reopened so that the GRL
+         won't proceed while write-locked tables are being reopened by other
+         threads.
+
+  @remark THD can be NULL, but then wait_for_refresh must be FALSE
+          and tables must be NULL.
 */
 
-bool close_cached_tables(THD *thd, bool if_wait_for_refresh,
-			 TABLE_LIST *tables, bool have_lock)
+bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
+                         bool wait_for_refresh, bool wait_for_placeholders)
 {
   bool result=0;
   DBUG_ENTER("close_cached_tables");
-  DBUG_ASSERT(thd || (!if_wait_for_refresh && !tables));
+  DBUG_ASSERT(thd || (!wait_for_refresh && !tables));
 
   if (!have_lock)
     VOID(pthread_mutex_lock(&LOCK_open));
@@ -941,7 +949,7 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh,
     }
     DBUG_PRINT("tcache", ("incremented global refresh_version to: %lu",
                           refresh_version));
-    if (if_wait_for_refresh)
+    if (wait_for_refresh)
     {
       /*
         Other threads could wait in a loop in open_and_lock_tables(),
@@ -998,13 +1006,13 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh,
 	found=1;
     }
     if (!found)
-      if_wait_for_refresh=0;			// Nothing to wait for
+      wait_for_refresh=0;			// Nothing to wait for
   }
 #ifndef EMBEDDED_LIBRARY
   if (!tables)
     kill_delayed_threads();
 #endif
-  if (if_wait_for_refresh)
+  if (wait_for_refresh)
   {
     /*
       If there is any table that has a lower refresh_version, wait until
@@ -1027,6 +1035,9 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh,
       for (uint idx=0 ; idx < open_cache.records ; idx++)
       {
 	TABLE *table=(TABLE*) hash_element(&open_cache,idx);
+        /* Avoid a self-deadlock. */
+        if (table->in_use == thd)
+          continue;
         /*
           Note that we wait here only for tables which are actually open, and
           not for placeholders with TABLE::open_placeholder set. Waiting for
@@ -1041,7 +1052,8 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh,
           are employed by CREATE TABLE as in this case table simply does not
           exist yet.
         */
-	if (table->needs_reopen_or_name_lock() && table->db_stat)
+	if (table->needs_reopen_or_name_lock() && (table->db_stat ||
+            (table->open_placeholder && wait_for_placeholders)))
 	{
 	  found=1;
           DBUG_PRINT("signal", ("Waiting for COND_refresh"));
@@ -1060,11 +1072,18 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh,
     thd->in_lock_tables=0;
     /* Set version for table */
     for (TABLE *table=thd->open_tables; table ; table= table->next)
-      table->s->version= refresh_version;
+    {
+      /*
+        Preserve the version (0) of write locked tables so that a impending
+        global read lock won't sneak in.
+      */
+      if (table->reginfo.lock_type < TL_WRITE_ALLOW_WRITE)
+        table->s->version= refresh_version;
+    }
   }
   if (!have_lock)
     VOID(pthread_mutex_unlock(&LOCK_open));
-  if (if_wait_for_refresh)
+  if (wait_for_refresh)
   {
     pthread_mutex_lock(&thd->mysys_var->mutex);
     thd->mysys_var->current_mutex= 0;
@@ -1091,10 +1110,10 @@ bool close_cached_connection_tables(THD *thd, bool if_wait_for_refresh,
   DBUG_ASSERT(thd);
 
   bzero(&tmp, sizeof(TABLE_LIST));
-  
+
   if (!have_lock)
     VOID(pthread_mutex_lock(&LOCK_open));
-  
+
   for (idx= 0; idx < table_def_cache.records; idx++)
   {
     TABLE_SHARE *share= (TABLE_SHARE *) hash_element(&table_def_cache, idx);
@@ -1123,11 +1142,11 @@ bool close_cached_connection_tables(THD *thd, bool if_wait_for_refresh,
   }
 
   if (tables)
-    result= close_cached_tables(thd, FALSE, tables, TRUE);
-  
+    result= close_cached_tables(thd, tables, TRUE, FALSE, FALSE);
+
   if (!have_lock)
     VOID(pthread_mutex_unlock(&LOCK_open));
-  
+
   if (if_wait_for_refresh)
   {
     pthread_mutex_lock(&thd->mysys_var->mutex);
@@ -2227,7 +2246,7 @@ void wait_for_condition(THD *thd, pthread_mutex_t *mutex, pthread_cond_t *cond)
   current thread.
 
   @param thd current thread context
-  @param tables able list containing one table to open.
+  @param tables table list containing one table to open.
 
   @return FALSE on success, TRUE otherwise.
 */
@@ -3295,8 +3314,8 @@ static bool reattach_merge(THD *thd, TABLE **err_tables_p)
 
     @param thd         Thread context
     @param get_locks   Should we get locks after reopening tables ?
-    @param in_refresh  Are we in FLUSH TABLES ? TODO: It seems that
-                       we can remove this parameter.
+    @param mark_share_as_old  Mark share as old to protect from a impending
+                              global read lock.
 
     @note Since this function can't properly handle prelocking and
           create placeholders it should be used in very special
@@ -3310,13 +3329,17 @@ static bool reattach_merge(THD *thd, TABLE **err_tables_p)
     @return FALSE in case of success, TRUE - otherwise.
 */
 
-bool reopen_tables(THD *thd,bool get_locks,bool in_refresh)
+bool reopen_tables(THD *thd, bool get_locks, bool mark_share_as_old)
 {
   TABLE *table,*next,**prev;
   TABLE **tables,**tables_ptr;			// For locks
   TABLE *err_tables= NULL;
   bool error=0, not_used;
   bool merge_table_found= FALSE;
+  const uint flags= MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN |
+                    MYSQL_LOCK_IGNORE_GLOBAL_READ_LOCK |
+                    MYSQL_LOCK_IGNORE_FLUSH;
+
   DBUG_ENTER("reopen_tables");
 
   if (!thd->open_tables)
@@ -3377,7 +3400,7 @@ bool reopen_tables(THD *thd,bool get_locks,bool in_refresh)
       /* Do not handle locks of MERGE children. */
       if (get_locks && !db_stat && !table->parent)
 	*tables_ptr++= table;			// need new lock on this
-      if (in_refresh)
+      if (mark_share_as_old)
       {
 	table->s->version=0;
 	table->open_placeholder= 0;
@@ -3410,7 +3433,7 @@ bool reopen_tables(THD *thd,bool get_locks,bool in_refresh)
     */
     thd->some_tables_deleted=0;
     if ((lock= mysql_lock_tables(thd, tables, (uint) (tables_ptr - tables),
-                                 MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN, &not_used)))
+                                 flags, &not_used)))
     {
       thd->locked_tables=mysql_lock_merge(thd->locked_tables,lock);
     }

sql/sql_parse.cc

@@ -6527,8 +6527,8 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
       tmp_write_to_binlog= 0;
       if (lock_global_read_lock(thd))
 	return 1;                               // Killed
-      result=close_cached_tables(thd,(options & REFRESH_FAST) ? 0 : 1,
-                                 tables);
+      result= close_cached_tables(thd, tables, FALSE, (options & REFRESH_FAST) ?
+                                  FALSE : TRUE, TRUE);
       if (make_global_read_lock_block_commit(thd)) // Killed
       {
         /* Don't leave things in a half-locked state */
@@ -6537,7 +6537,8 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
       }
     }
     else
-      result=close_cached_tables(thd,(options & REFRESH_FAST) ? 0 : 1, tables);
+      result= close_cached_tables(thd, tables, FALSE, (options & REFRESH_FAST) ?
+                                  FALSE : TRUE, FALSE);
     my_dbopt_cleanup();
   }
   if (options & REFRESH_HOSTS)

sql/sql_table.cc

@@ -6690,7 +6690,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   if (thd->locked_tables && new_name == table_name && new_db == db)
   {
     thd->in_lock_tables= 1;
-    error= reopen_tables(thd, 1, 0);
+    error= reopen_tables(thd, 1, 1);
     thd->in_lock_tables= 0;
     if (error)
       goto err_with_placeholders;