MDEV-20945: BACKUP UNLOCK + FTWRL assertion failure

MDEV-20945: BACKUP UNLOCK + FTWRL assertion failure | SIGSEGV in I_P_List
from MDL_context::release_lock on INSERT w/ BACKUP LOCK (on optimized
builds) | Assertion `ticket->m_duration == MDL_EXPLICIT' failed

BACKUP LOCK behavior is modified so it won't be used wrong:
- BACKUP LOCK should commit any active transactions.
- BACKUP LOCK should not be allowed in stored procedures.
- When BACKUP LOCK is active, don't allow any DDL's for that connection.
- FTWRL is forbidden on the same connection while BACKUP LOCK is active.

Reviewed-by: monty@mariadb.com

mysql-test/main/backup_locks.result

@@ -39,6 +39,108 @@ MDL_INTENTION_EXCLUSIVE	Schema metadata lock	test
 select * from t1;
 ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
 backup unlock;
+#
+# BACKUP LOCK and BACKUP UNLOCK are not allowed in procedures.
+#
+CREATE PROCEDURE p_BACKUP_LOCK()
+BEGIN
+BACKUP LOCK;
+END|
+ERROR 0A000: BACKUP LOCK is not allowed in stored procedures
+CREATE PROCEDURE p_BACKUP_UNLOCK()
+BEGIN
+BACKUP UNLOCK;
+END|
+ERROR 0A000: BACKUP UNLOCK is not allowed in stored procedures
+#
+# BACKUP STAGE doesn't work when a BACKUP LOCK is active.
+#
+CREATE TABLE t1 (a INT);
+BACKUP LOCK t1;
+BACKUP STAGE START;
+ERROR HY000: Can't execute the query because you have a conflicting read lock
+BACKUP UNLOCK;
+DROP TABLE t1;
+#
+# FLUSH TABLES WITH READ LOCK is not allowed when BACKUP LOCK is active.
+#
+CREATE TABLE t1 (a INT);
+BACKUP LOCK t1;
+FLUSH TABLES t1 WITH READ LOCK;
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+BACKUP UNLOCK;
+BACKUP LOCK t1;
+FLUSH TABLES WITH READ LOCK;
+BACKUP UNLOCK;
+UNLOCK TABLES;
+DROP TABLE t1;
+#
+# MDEV-20945 BACKUP UNLOCK assertion failures.
+#
+# Scenario 1.
+CREATE TABLE t1 (a INT);
+BACKUP LOCK t1;
+FLUSH TABLE t1 WITH READ LOCK;
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+UNLOCK TABLES;
+BACKUP UNLOCK;
+DROP TABLE t1;
+# Scenario 2.
+CREATE TABLE t1 (a INT);
+CREATE TABLE t2 (b INT);
+LOCK TABLES t2 AS a2 WRITE;
+BACKUP LOCK t1;
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+UNLOCK TABLES;
+INSERT INTO t1 VALUES(0);
+# restart
+DROP TABLE t1;
+DROP TABLE t2;
+# Scenario 3.
+CREATE TEMPORARY TABLE t3 (c INT);
+BACKUP LOCK t1;
+SET @@SESSION.profiling=ON;
+CREATE TABLE t1 (c INT);
+ERROR HY000: Can't execute the query because you have a conflicting read lock
+LOCK TABLES t3 AS a1 READ, t1 AS a3 READ, t3 AS a5 READ LOCAL;
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+UNLOCK TABLE;
+# restart
+# Scenario 4.
+CREATE TABLE t (c INT);
+BACKUP LOCK not_existing.t;
+LOCK TABLES t WRITE;
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+UNLOCK TABLES;
+# restart
+DROP TABLE t;
+# Scenario 5.
+BACKUP LOCK t1;
+CREATE TABLE t2 (c1 TIME, c2 TIME, c3 DATE, KEY(c1, c2));
+ERROR HY000: Can't execute the query because you have a conflicting read lock
+LOCK TABLE t2 READ;
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+# restart
+# Scenario 6.
+BACKUP LOCK t;
+CREATE VIEW v AS SELECT 1;
+ERROR HY000: Can't execute the query because you have a conflicting read lock
+LOCK TABLES v READ;
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+START TRANSACTION READ ONLY;
+BACKUP LOCK t;
+# restart
+# Scenario 7.
+SET SQL_MODE='';
+SET STATEMENT max_statement_time=180 FOR BACKUP LOCK test.u;
+CREATE TABLE t (a INT) ENGINE=Aria;
+ERROR HY000: Can't execute the query because you have a conflicting read lock
+CREATE TEMPORARY TABLE IF NOT EXISTS s (c INT) ENGINE=Aria;
+LOCK TABLES s AS a READ LOCAL,t AS b WRITE;
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+SET STATEMENT max_statement_time=180 FOR BACKUP LOCK test.u;
+# restart
+#
 connection con1;
 connection default;
 disconnect con1;

mysql-test/main/backup_locks.test

@@ -43,6 +43,125 @@ SELECT LOCK_MODE, LOCK_TYPE, TABLE_SCHEMA, TABLE_NAME FROM information_schema.me
 --error ER_LOCK_DEADLOCK
 select * from t1;
 backup unlock;
+
+--echo #
+--echo # BACKUP LOCK and BACKUP UNLOCK are not allowed in procedures.
+--echo #
+delimiter |;
+--error ER_SP_BADSTATEMENT
+CREATE PROCEDURE p_BACKUP_LOCK()
+BEGIN
+  BACKUP LOCK;
+END|
+--error ER_SP_BADSTATEMENT
+CREATE PROCEDURE p_BACKUP_UNLOCK()
+BEGIN
+  BACKUP UNLOCK;
+END|
+delimiter ;|
+
+--echo #
+--echo # BACKUP STAGE doesn't work when a BACKUP LOCK is active.
+--echo #
+CREATE TABLE t1 (a INT);
+BACKUP LOCK t1;
+--error ER_CANT_UPDATE_WITH_READLOCK
+BACKUP STAGE START;
+BACKUP UNLOCK;
+DROP TABLE t1;
+
+--echo #
+--echo # FLUSH TABLES WITH READ LOCK is not allowed when BACKUP LOCK is active.
+--echo #
+CREATE TABLE t1 (a INT);
+BACKUP LOCK t1;
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+FLUSH TABLES t1 WITH READ LOCK;
+BACKUP UNLOCK;
+
+BACKUP LOCK t1;
+FLUSH TABLES WITH READ LOCK;
+BACKUP UNLOCK;
+UNLOCK TABLES;
+DROP TABLE t1;
+
+--echo #
+--echo # MDEV-20945 BACKUP UNLOCK assertion failures.
+--echo #
+
+--echo # Scenario 1.
+CREATE TABLE t1 (a INT);
+BACKUP LOCK t1;
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+FLUSH TABLE t1 WITH READ LOCK;
+UNLOCK TABLES;
+BACKUP UNLOCK;   # Shouldn't trigger an assertion.
+DROP TABLE t1;
+
+--echo # Scenario 2.
+CREATE TABLE t1 (a INT);
+CREATE TABLE t2 (b INT);
+LOCK TABLES t2 AS a2 WRITE;
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+BACKUP LOCK t1;
+UNLOCK TABLES;
+INSERT INTO t1 VALUES(0);
+--source include/restart_mysqld.inc
+DROP TABLE t1;
+DROP TABLE t2;
+
+--echo # Scenario 3.
+CREATE TEMPORARY TABLE t3 (c INT);
+BACKUP LOCK t1;  # Table `t1` doesn't exist.
+SET @@SESSION.profiling=ON;
+--error ER_CANT_UPDATE_WITH_READLOCK
+CREATE TABLE t1 (c INT);
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+LOCK TABLES t3 AS a1 READ, t1 AS a3 READ, t3 AS a5 READ LOCAL;
+UNLOCK TABLE;
+--source include/restart_mysqld.inc
+
+--echo # Scenario 4.
+CREATE TABLE t (c INT);
+BACKUP LOCK not_existing.t;
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+LOCK TABLES t WRITE;
+UNLOCK TABLES;
+--source include/restart_mysqld.inc
+DROP TABLE t;
+
+--echo # Scenario 5.
+BACKUP LOCK t1;
+--error ER_CANT_UPDATE_WITH_READLOCK
+CREATE TABLE t2 (c1 TIME, c2 TIME, c3 DATE, KEY(c1, c2));
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+LOCK TABLE t2 READ;
+--source include/restart_mysqld.inc
+
+--echo # Scenario 6.
+BACKUP LOCK t;
+--error ER_CANT_UPDATE_WITH_READLOCK
+CREATE VIEW v AS SELECT 1;
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+LOCK TABLES v READ;
+START TRANSACTION READ ONLY;
+BACKUP LOCK t;
+--source include/restart_mysqld.inc
+
+--echo # Scenario 7.
+SET SQL_MODE='';
+SET STATEMENT max_statement_time=180 FOR BACKUP LOCK test.u;
+--error ER_CANT_UPDATE_WITH_READLOCK
+CREATE TABLE t (a INT) ENGINE=Aria;
+CREATE TEMPORARY TABLE IF NOT EXISTS s (c INT) ENGINE=Aria;
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+LOCK TABLES s AS a READ LOCAL,t AS b WRITE;
+--let $q= `SELECT * FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE`
+SET STATEMENT max_statement_time=180 FOR BACKUP LOCK test.u;
+--source include/restart_mysqld.inc
+
+--echo #
+
 connection con1;
 --reap
 connection default;

mysql-test/main/implicit_commit.result

@@ -1035,6 +1035,16 @@ YES
 # SQLCOM_BACKUP
 #
 #
+# SQLCOM_BACKUP_LOCK
+#
+INSERT INTO db1.trans VALUES (1);
+BACKUP LOCK t1;
+ROLLBACK;
+BACKUP UNLOCK;
+CALL db1.test_if_commit();
+IMPLICIT COMMIT
+YES
+#
 # SQLCOM_SHOW_ARCHIVE
 #
 #

mysql-test/main/implicit_commit.test

@@ -1114,6 +1114,16 @@ source include/implicit_commit_helper.inc;
 #  backup database db1 to 'backup_db1.ba';
 #source include/implicit_commit_helper.inc;
 
+--echo #
+--echo # SQLCOM_BACKUP_LOCK
+--echo #
+
+INSERT INTO db1.trans VALUES (1);
+BACKUP LOCK t1;
+ROLLBACK;
+BACKUP UNLOCK;
+CALL db1.test_if_commit();
+
 --echo #
 --echo # SQLCOM_SHOW_ARCHIVE
 --echo #

sql/backup.cc

@@ -377,7 +377,13 @@ bool backup_reset_alter_copy_lock(THD *thd)
 
 bool backup_lock(THD *thd, TABLE_LIST *table)
 {
+  /* We should leave the previous table unlocked in case of errors */
   backup_unlock(thd);
+  if (thd->locked_tables_mode)
+  {
+    my_error(ER_LOCK_OR_ACTIVE_TRANSACTION, MYF(0));
+    return 1;
+  }
   table->mdl_request.duration= MDL_EXPLICIT;
   if (thd->mdl_context.acquire_lock(&table->mdl_request,
                                     thd->variables.lock_wait_timeout))

sql/sql_class.cc

@@ -2569,7 +2569,7 @@ void THD::give_protection_error()
     my_error(ER_BACKUP_LOCK_IS_ACTIVE, MYF(0));
   else
   {
-    DBUG_ASSERT(global_read_lock.is_acquired());
+    DBUG_ASSERT(global_read_lock.is_acquired() || mdl_backup_lock);
     my_error(ER_CANT_UPDATE_WITH_READLOCK, MYF(0));
   }
 }

sql/sql_class.h

@@ -3630,10 +3630,17 @@ class THD: public THD_count, /* this must be first */
     return server_status & SERVER_STATUS_IN_TRANS;
   }
   void give_protection_error();
+  /*
+    Give an error if any of the following is true for this connection
+    - BACKUP STAGE is active
+    - FLUSH TABLE WITH READ LOCK is active
+    - BACKUP LOCK table_name is active
+  */
   inline bool has_read_only_protection()
   {
     if (current_backup_stage == BACKUP_FINISHED &&
-        !global_read_lock.is_acquired())
+        !global_read_lock.is_acquired() &&
+        !mdl_backup_lock)
       return FALSE;
     give_protection_error();
     return TRUE;

sql/sql_parse.cc

@@ -780,7 +780,7 @@ void init_update_queries(void)
   sql_command_flags[SQLCOM_ALTER_SERVER]=       CF_AUTO_COMMIT_TRANS;
   sql_command_flags[SQLCOM_DROP_SERVER]=        CF_AUTO_COMMIT_TRANS;
   sql_command_flags[SQLCOM_BACKUP]=             CF_AUTO_COMMIT_TRANS;
-  sql_command_flags[SQLCOM_BACKUP_LOCK]=        0;
+  sql_command_flags[SQLCOM_BACKUP_LOCK]=        CF_AUTO_COMMIT_TRANS;
 
   /*
     The following statements can deal with temporary tables,
@@ -5002,6 +5002,13 @@ mysql_execute_command(THD *thd)
       goto error;
     }
 
+    /* Should not lock tables while BACKUP LOCK is active */
+    if (thd->mdl_backup_lock)
+    {
+      my_error(ER_LOCK_OR_ACTIVE_TRANSACTION, MYF(0));
+      goto error;
+    }
+
     /*
       Here we have to pre-open temporary tables for LOCK TABLES.
 

sql/sql_reload.cc

@@ -217,6 +217,7 @@ bool reload_acl_and_cache(THD *thd, unsigned long long options,
               thd->handler_tables_hash.records ||
               thd->ull_hash.records ||
               thd->global_read_lock.is_acquired() ||
+              thd->mdl_backup_lock ||
               thd->current_backup_stage != BACKUP_FINISHED
               );
 
@@ -530,7 +531,14 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables)
   if (thd->current_backup_stage != BACKUP_FINISHED)
   {
     my_error(ER_BACKUP_LOCK_IS_ACTIVE, MYF(0));
-    return true;
+    goto error;
+  }
+
+  /* Should not flush tables while BACKUP LOCK is active */
+  if (thd->mdl_backup_lock)
+  {
+    my_error(ER_LOCK_OR_ACTIVE_TRANSACTION, MYF(0));
+    goto error;
   }
 
   if (thd->lex->type & REFRESH_READ_LOCK)

sql/sql_yacc.yy

@@ -14868,6 +14868,8 @@ backup_statements:
         }
 	| LOCK_SYM
           {
+            if (unlikely(Lex->sphead))
+              my_yyabort_error((ER_SP_BADSTATEMENT, MYF(0), "BACKUP LOCK"));
             if (Lex->main_select_push())
               MYSQL_YYABORT;
           }
@@ -14881,6 +14883,8 @@ backup_statements:
           }
         | UNLOCK_SYM
           {
+            if (unlikely(Lex->sphead))
+              my_yyabort_error((ER_SP_BADSTATEMENT, MYF(0), "BACKUP UNLOCK"));
 	    /* Table list is empty for unlock */
             Lex->sql_command= SQLCOM_BACKUP_LOCK;
           }