Bug#21808680: JSON + GENERATED COLUMN CORRUPTS TABLE CACHE MEMORY, CRASHES

In some cases pointers to stale data could be left in the Item tree of
the generated column expression, which would lead to problems the next
time the generated column was accessed.

There is already code in place to avoid this situation.
TABLE::cleanup_gc_items() calls Item::cleanup() on the generated
column expression upon completion of a statement, and this is supposed
to revert the Item tree to a pristine state, ready for reuse in a new
statement.

However, since the Item::cleanup() functions are not recursive, only
the top-level item in the generated column expression will be cleaned
up. Children, such as items representing the arguments of a function
call, will not be cleaned up and can point to stale data inserted into
the item tree during execution.

The fix changes TABLE::cleanup_gc_items() so that it calls cleanup()
on all the items in the generated column's item_free_list, and not
only on the top-level item.

The patch also removes an unused parameter from the function.

Author: kahatlen

mysql-test/suite/gcol/r/gcol_bugfixes.result

@@ -492,3 +492,14 @@ c13
 00:00:01
 DROP TABLE C;
 DROP VIEW view_C;
+#
+# Bug #21808680: JSON + GENERATED COLUMN CORRUPTS TABLE CACHE
+#                MEMORY, CRASHES
+#
+CREATE TABLE t (a INT, b JSON, c TEXT GENERATED ALWAYS AS (REPEAT(a=b, 2)));
+INSERT INTO t (a, b) VALUES (1, '2'), (3, '3');
+SELECT * FROM t;
+a	b	c
+1	2	00
+3	3	11
+DROP TABLE t;

mysql-test/suite/gcol/t/gcol_bugfixes.test

@@ -485,3 +485,13 @@ SELECT DISTINCT t1.c13 FROM C AS t1, view_C AS t2;
 
 DROP TABLE C;
 DROP VIEW view_C;
+
+--echo #
+--echo # Bug #21808680: JSON + GENERATED COLUMN CORRUPTS TABLE CACHE
+--echo #                MEMORY, CRASHES
+--echo #
+CREATE TABLE t (a INT, b JSON, c TEXT GENERATED ALWAYS AS (REPEAT(a=b, 2)));
+INSERT INTO t (a, b) VALUES (1, '2'), (3, '3');
+# The next statement used to crash.
+SELECT * FROM t;
+DROP TABLE t;

sql/sql_base.cc

@@ -1337,7 +1337,7 @@ static void mark_temp_tables_as_free_for_reuse(THD *thd)
     if ((table->query_id == thd->query_id) && ! table->open_by_handler)
     {
       mark_tmp_table_for_reuse(table);
-      table->cleanup_gc_items(thd);
+      table->cleanup_gc_items();
     }
   }
 }
@@ -1567,7 +1567,7 @@ void close_thread_tables(THD *thd)
     {
       DBUG_ASSERT(table->file);
       table->file->extra(HA_EXTRA_DETACH_CHILDREN);
-      table->cleanup_gc_items(thd);
+      table->cleanup_gc_items();
     }
   }
 

sql/table.cc

@@ -4646,17 +4646,13 @@ bool TABLE::refix_gc_items(THD *thd)
 }
 
 
-void TABLE::cleanup_gc_items(THD *thd)
+void TABLE::cleanup_gc_items()
 {
-  if (vfield)
-  {
-    for (Field **vfield_ptr= vfield; *vfield_ptr; vfield_ptr++)
-    {
-      Field *vfield= *vfield_ptr;
-      DBUG_ASSERT(vfield->gcol_info && vfield->gcol_info->expr_item);
-      vfield->gcol_info->expr_item->cleanup();
-    }
-  }
+  if (!has_gcol())
+    return;
+
+  for (Field **vfield_ptr= vfield; *vfield_ptr; vfield_ptr++)
+    cleanup_items((*vfield_ptr)->gcol_info->item_free_list);
 }
 
 /*

sql/table.h

@@ -1426,10 +1426,8 @@ struct TABLE
   /**
     Clean any state in items associated with generated columns to be ready for
     the next statement.
-   
-    @param[in] thd    the current thread
   */
-  void cleanup_gc_items(THD *thd);
+  void cleanup_gc_items();
 };