MDEV-30955 Explicit locks released too early in rollback path

Assertion `thd->mdl_context.is_lock_owner()` fires when a client is
disconnected, while transaction and and a table is opened through
`HANDLER` interface.
Reason for the assertion is that when a connection closes, its ongoing
transaction is eventually rolled back in
`Wsrep_client_state::bf_rollback()`. This method also releases explicit
which are expected to survive beyond the transaction lifetime.
This patch also removes calls to `mysql_ull_cleanup()`. User level
locks are not supported in combination with Galera, making these calls
unnecessary.

5 files changed, 97 insertions, 5 deletions

diff --git a/mysql-test/suite/galera/r/MDEV-30955.result b/mysql-test/suite/galera/r/MDEV-30955.result
new file mode 100644
index 00000000000..2a090cb58bc
--- /dev/null
+++ b/mysql-test/suite/galera/r/MDEV-30955.result
@@ -0,0 +1,26 @@
+connection node_2;
+connection node_1;
+CREATE TABLE t (a CHAR(1) KEY);
+START TRANSACTION;
+HANDLER t OPEN;
+disconnect node_1;
+connect node_1, 127.0.0.1, root, , test, $NODE_MYPORT_1;
+DROP TABLE t;
+BACKUP STAGE START;
+START TRANSACTION;
+disconnect node_1;
+connect node_1, 127.0.0.1, root, , test, $NODE_MYPORT_1;
+connection node_1;
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+CREATE TABLE t2 (f1 INTEGER PRIMARY KEY);
+START TRANSACTION;
+INSERT INTO t1 VALUES(1);
+HANDLER t2 OPEN;
+connection node_2;
+INSERT INTO t1 VALUES(1);
+connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1;
+connection node_1a;
+connection node_1;
+COMMIT;
+ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+DROP TABLE t1,t2;
diff --git a/mysql-test/suite/galera/t/MDEV-30955.test b/mysql-test/suite/galera/t/MDEV-30955.test
new file mode 100644
index 00000000000..18577120e83
--- /dev/null
+++ b/mysql-test/suite/galera/t/MDEV-30955.test
@@ -0,0 +1,70 @@
+#
+# MDEV-30955
+# Assertion `thd->mdl_context.is_lock_owner(MDL_key::TABLE,
+# table->s->db.str, table->s->table_name.str, MDL_SHARED)'
+# failed in close_thread_table()
+#
+
+--source include/galera_cluster.inc
+
+#
+# Test 1: Assertion thd->mdl_context.is_lock_owner()
+# failed in close_thread_table()
+#
+CREATE TABLE t (a CHAR(1) KEY);
+START TRANSACTION;
+HANDLER t OPEN;
+
+#
+# If bug is present the transaction will be aborted
+# through Wsrep_client_service::bf_rollback() and
+# release explicit locks too early. Later, during
+# THD::cleanup(), table t will be closed and the
+# THD is expected to be owner of the MDL lock that
+# was just released.
+#
+--disconnect node_1
+
+--connect node_1, 127.0.0.1, root, , test, $NODE_MYPORT_1
+DROP TABLE t;
+
+
+#
+# Test 2: Similar issue reproduces also with BACKUP STAGE locks.
+# See comments in MDEV-25037
+#
+
+BACKUP STAGE START;
+START TRANSACTION;
+--disconnect node_1
+--connect node_1, 127.0.0.1, root, , test, $NODE_MYPORT_1
+
+
+#
+# Test 3: Assertion `!thd->mdl_context.has_locks()' failed
+# in do_command()
+#
+
+--connection node_1
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+CREATE TABLE t2 (f1 INTEGER PRIMARY KEY);
+
+--let $bf_count = `SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.global_status WHERE VARIABLE_NAME = 'wsrep_local_bf_aborts'`
+
+START TRANSACTION;
+INSERT INTO t1 VALUES(1);
+HANDLER t2 OPEN;
+
+--connection node_2
+INSERT INTO t1 VALUES(1);
+
+--connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1
+--connection node_1a
+--let $wait_condition = SELECT VARIABLE_VALUE = $bf_count + 1 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_local_bf_aborts'
+--source include/wait_condition.inc
+
+--connection node_1
+--error ER_LOCK_DEADLOCK
+COMMIT;
+
+DROP TABLE t1,t2;
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index c495ae2d6c4..c95993e1604 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -1309,7 +1309,7 @@ bool do_command(THD *thd)
       in wsrep_before_command().
     */
     WSREP_LOG_THD(thd, "enter found BF aborted");
-    DBUG_ASSERT(!thd->mdl_context.has_locks());
+    DBUG_ASSERT(!thd->mdl_context.has_transactional_locks());
     DBUG_ASSERT(!thd->get_stmt_da()->is_set());
     /* We let COM_QUIT and COM_STMT_CLOSE to execute even if wsrep aborted. */
     if (command == COM_STMT_EXECUTE)
diff --git a/sql/wsrep_client_service.cc b/sql/wsrep_client_service.cc
index f00dfccf274..0399cf4f442 100644
--- a/sql/wsrep_client_service.cc
+++ b/sql/wsrep_client_service.cc
@@ -362,8 +362,6 @@ int Wsrep_client_service::bf_rollback()
     m_thd->global_read_lock.unlock_global_read_lock(m_thd);
   }
   m_thd->release_transactional_locks();
-  mysql_ull_cleanup(m_thd);
-  m_thd->mdl_context.release_explicit_locks();
 
   DBUG_RETURN(ret);
 }
diff --git a/sql/wsrep_high_priority_service.cc b/sql/wsrep_high_priority_service.cc
index 96269481559..3c6524b7ddf 100644
--- a/sql/wsrep_high_priority_service.cc
+++ b/sql/wsrep_high_priority_service.cc
@@ -357,8 +357,6 @@ int Wsrep_high_priority_service::rollback(const wsrep::ws_handle& ws_handle,
   m_thd->wsrep_cs().prepare_for_ordering(ws_handle, ws_meta, false);
   int ret= (trans_rollback_stmt(m_thd) || trans_rollback(m_thd));
   m_thd->release_transactional_locks();
-  mysql_ull_cleanup(m_thd);
-  m_thd->mdl_context.release_explicit_locks();
 
   free_root(m_thd->mem_root, MYF(MY_KEEP_PREALLOC));