From 6f78efc01c62d57e63632c5c9a3f283dd1f81642 Mon Sep 17 00:00:00 2001
From: Nikita Malyavin <nikitamalyavin@gmail.com>
Date: Wed, 22 Mar 2023 23:31:15 +0300
Subject: [PATCH] MDEV-30902 Server crash in LEX::first_lists_tables_same

ONLINE ALTER TABLE uses binlog events like the replication does.

Before it was never used outside of replication, so significant
change was required. For example, a single event had a statement-like
befavior: it locked the tables, opened it, and closed them in the end. But
for ONLINE ALTER we use preopened table.

A crash scenario is following: lex->query_tables was set to NULL in
restore_empty_query_table_list when alter event is applied.
Then lex->query_tables->prev_global was write-accessed in
LEX::first_lists_tables_same, leading to a segfault.

In replication restore_empty_query_table_list would mean resetting lex
before next query or event.

In ONLINE ALTER TABLE we reuse a locked table between the events, so
we should avoid it. Here the need to reset lex state (or close the tables)
can be determined by nonzero rgi->tables_to_lock_count.
If no table is locked, then event doesn't own the tables.

The same was already done before for rgi->slave_close_thread_tables call.
---
 .../main/alter_table_online_debug.result      | 18 +++++++++++++
 mysql-test/main/alter_table_online_debug.test | 25 +++++++++++++++++++
 sql/log_event_server.cc                       | 13 +++++++---
 3 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/mysql-test/main/alter_table_online_debug.result b/mysql-test/main/alter_table_online_debug.result
index b52187bfc4c..b76abdaf679 100644
--- a/mysql-test/main/alter_table_online_debug.result
+++ b/mysql-test/main/alter_table_online_debug.result
@@ -976,6 +976,8 @@ insert into t values (3);
 set debug_sync= 'now signal goforit';
 xa end 'xid';
 xa commit 'xid' one phase;
+connection con1;
+connection default;
 drop table t;
 set debug_sync= reset;
 #
@@ -1158,5 +1160,21 @@ set debug_sync= reset;
 set debug_dbug= @old_debug;
 connection default;
 #
+# MDEV-30902 Server crash in LEX::first_lists_tables_same
+#
+create table t1 engine=myisam select 1 as x ;
+create procedure p() alter table t1 engine=heap;
+set debug_sync= 'alter_table_copy_end signal ended wait_for end';
+call p;
+connection con1;
+set debug_sync= 'now wait_for ended';
+insert into t1 values (2);
+set debug_sync= 'now signal end';
+connection default;
+call p;
+drop table t1;
+drop procedure p;
+set debug_sync=reset;
+#
 # End of 11.2 tests
 #
diff --git a/mysql-test/main/alter_table_online_debug.test b/mysql-test/main/alter_table_online_debug.test
index f9cadcb59cf..a2f0f9ca0ef 100644
--- a/mysql-test/main/alter_table_online_debug.test
+++ b/mysql-test/main/alter_table_online_debug.test
@@ -1149,6 +1149,9 @@ xa end 'xid';
 xa commit 'xid' one phase;
 
 # Cleanup
+--connection con1
+--reap
+--connection default
 drop table t;
 set debug_sync= reset;
 
@@ -1326,6 +1329,28 @@ set debug_sync= reset;
 set debug_dbug= @old_debug;
 --connection default
 
+--echo #
+--echo # MDEV-30902 Server crash in LEX::first_lists_tables_same
+--echo #
+create table t1 engine=myisam select 1 as x ;
+create procedure p() alter table t1 engine=heap;
+
+set debug_sync= 'alter_table_copy_end signal ended wait_for end';
+send call p;
+
+--connection con1
+set debug_sync= 'now wait_for ended';
+insert into t1 values (2);
+set debug_sync= 'now signal end';
+
+--connection default
+--reap
+call p;
+
+drop table t1;
+drop procedure p;
+set debug_sync=reset;
+
 --echo #
 --echo # End of 11.2 tests
 --echo #
diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc
index 0e994a456b0..7398ae8e7fb 100644
--- a/sql/log_event_server.cc
+++ b/sql/log_event_server.cc
@@ -5238,8 +5238,13 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi)
     goto err;
   }
 
-  /* remove trigger's tables */
-  restore_empty_query_table_list(thd->lex);
+  /*
+    Remove trigger's tables. In case of ONLINE ALTER TABLE, event doesn't own
+    the table (hence, no tables are locked), and therefore no cleanup should be
+    done after each event.
+  */
+  if (rgi->tables_to_lock_count)
+    restore_empty_query_table_list(thd->lex);
 
   if (WSREP(thd) && wsrep_thd_is_applying(thd))
     query_cache_invalidate_locked_for_write(thd, rgi->tables_to_lock);
@@ -5258,9 +5263,11 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi)
   DBUG_RETURN(error);
 
 err:
-  restore_empty_query_table_list(thd->lex);
   if (rgi->tables_to_lock_count)
+  {
+    restore_empty_query_table_list(thd->lex);
     rgi->slave_close_thread_tables(thd);
+  }
   thd->reset_query_timer();
   DBUG_RETURN(error);
 }