Bug#48157: crash in Item_field::used_tables

MySQL handles the join syntax "JOIN ... USING( field1,
... )" and natural joins by building the same parse tree as
a corresponding join with an "ON t1.field1 = t2.field1 ..."
expression would produce. This parse tree was not cleaned up
properly in the following scenario. If a thread tries to
lock some tables and finds that the tables were dropped and
re-created while waiting for the lock, it cleans up column
references in the statement by means a per-statement free
list. But if the statement was part of a stored procedure,
column references on the stored procedure's free list weren't
cleaned up and thus contained pointers to freed objects.

Fixed by adding a call to clean up the current prepared
statement's free list.


mysql-test/r/sp_sync.result:
  Bug#48157: Test case
mysql-test/t/sp_sync.test:
  Bug#48157: Test result
sql/item.h:
  Bug#48157: Commented field.
sql/sql_parse.cc:
  Bug#48157: Commented function.
sql/sql_update.cc:
  Bug#48157: fix

5 files changed, 94 insertions, 3 deletions

diff --git a/mysql-test/r/sp_sync.result b/mysql-test/r/sp_sync.result
new file mode 100644
index 00000000000..afa37e70531
--- /dev/null
+++ b/mysql-test/r/sp_sync.result
@@ -0,0 +1,23 @@
+Tests of syncronization of stored procedure execution.
+#
+# Bug#48157: crash in Item_field::used_tables
+#
+CREATE TABLE t1 AS SELECT 1 AS a, 1 AS b;
+CREATE TABLE t2 AS SELECT 1 AS a, 1 AS b;
+CREATE PROCEDURE p1()
+BEGIN
+UPDATE t1 JOIN t2 USING( a, b ) SET t1.b = 1, t2.b = 1;
+END|
+LOCK TABLES t1 WRITE, t2 WRITE;
+SET DEBUG_SYNC = 'multi_update_reopen_tables SIGNAL parked WAIT_FOR go';
+CALL p1();
+DROP TABLE t1, t2;
+SET DEBUG_SYNC = 'now WAIT_FOR parked';
+CREATE TABLE t1 AS SELECT 1 AS a, 1 AS b;
+CREATE TABLE t2 AS SELECT 1 AS a, 1 AS b;
+SET DEBUG_SYNC = 'now SIGNAL go';
+# Without the DEBUG_SYNC supplied in the same patch as this test in the 
+# code, this test statement will hang.
+DROP TABLE t1, t2;
+DROP PROCEDURE p1;
+SET DEBUG_SYNC = 'RESET';
diff --git a/mysql-test/t/sp_sync.test b/mysql-test/t/sp_sync.test
new file mode 100644
index 00000000000..98903989cd5
--- /dev/null
+++ b/mysql-test/t/sp_sync.test
@@ -0,0 +1,55 @@
+--echo Tests of syncronization of stored procedure execution.
+
+--source include/have_debug_sync.inc
+
+--echo #
+--echo # Bug#48157: crash in Item_field::used_tables
+--echo #
+
+CREATE TABLE t1 AS SELECT 1 AS a, 1 AS b;
+CREATE TABLE t2 AS SELECT 1 AS a, 1 AS b;
+
+DELIMITER |;
+
+CREATE PROCEDURE p1()
+BEGIN
+   UPDATE t1 JOIN t2 USING( a, b ) SET t1.b = 1, t2.b = 1;
+END|
+
+DELIMITER ;|
+
+connect (con1,localhost,root,,);
+connect (con2,localhost,root,,);
+
+connection con1;
+LOCK TABLES t1 WRITE, t2 WRITE;
+
+connection con2;
+LET $ID= `select connection_id()`;
+SET DEBUG_SYNC = 'multi_update_reopen_tables SIGNAL parked WAIT_FOR go';
+--send CALL p1()
+
+connection con1;
+let $wait_condition= SELECT 1 FROM information_schema.processlist WHERE ID = $ID AND
+state = "Locked";
+--source include/wait_condition.inc
+DROP TABLE t1, t2;
+SET DEBUG_SYNC = 'now WAIT_FOR parked';
+CREATE TABLE t1 AS SELECT 1 AS a, 1 AS b;
+CREATE TABLE t2 AS SELECT 1 AS a, 1 AS b;
+SET DEBUG_SYNC = 'now SIGNAL go';
+
+connection con2;
+--reap
+
+disconnect con1;
+disconnect con2;
+connection default;
+
+--echo # Without the DEBUG_SYNC supplied in the same patch as this test in the 
+--echo # code, this test statement will hang.
+DROP TABLE t1, t2;
+DROP PROCEDURE p1;
+
+SET DEBUG_SYNC = 'RESET';
+
diff --git a/sql/item.h b/sql/item.h
index 8f0e5874f3f..88e90924fcc 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -506,6 +506,13 @@ public:
   char * name;			/* Name from select */
   /* Original item name (if it was renamed)*/
   char * orig_name;
+  /**
+     Intrusive list pointer for free list. If not null, points to the next
+     Item on some Query_arena's free list. For instance, stored procedures
+     have their own Query_arena's.
+
+     @see Query_arena::free_list
+   */
   Item *next;
   uint32 max_length;
   uint name_length;                     /* Length of name */
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 69c9ddc7806..48743a2d48f 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -615,8 +615,10 @@ void free_items(Item *item)
   DBUG_VOID_RETURN;
 }
 
-/* This works because items are allocated with sql_alloc() */
-
+/**
+   This works because items are allocated with sql_alloc().
+   @note The function also handles null pointers (empty list).
+*/
 void cleanup_items(Item *item)
 {
   DBUG_ENTER("cleanup_items");  
diff --git a/sql/sql_update.cc b/sql/sql_update.cc
index c988d746500..26f40c7fa9f 100644
--- a/sql/sql_update.cc
+++ b/sql/sql_update.cc
@@ -23,6 +23,7 @@
 #include "sql_select.h"
 #include "sp_head.h"
 #include "sql_trigger.h"
+#include "debug_sync.h"
 
 /* Return 0 if row hasn't changed */
 
@@ -1143,8 +1144,11 @@ reopen_tables:
       items from 'fields' list, so the cleanup above is necessary to.
     */
     cleanup_items(thd->free_list);
-
+    cleanup_items(thd->stmt_arena->free_list);
     close_tables_for_reopen(thd, &table_list);
+
+    DEBUG_SYNC(thd, "multi_update_reopen_tables");
+
     goto reopen_tables;
   }