MDEV-32733: Two JSON related tests running in PS mode fail on server built...

MDEV-32733: Two JSON related tests running in PS mode fail on server built with -DWITH_PROTECT_STATEMENT_MEMROOT=YES

The tests main.func_json and json.json_no_table fail on server built with
the option -DWITH_PROTECT_STATEMENT_MEMROOT=YES by the reason that a memory
is allocated on the statement's memory root on the second execution of
a query that uses the function json_contains_path().

The reason that a memory is allocated on second execution of a prepared
statement that ivokes the function json_contains_path() is that a memory
allocated on every call of the method Item_json_str_multipath::fix_fields

To fix the issue, memory allocation should be done only once on first
call of the method Item_json_str_multipath::fix_fields. Simmilar issue
take place inside the method Item_func_json_contains_path::fix_fields.
Both methods are modified to make memory allocation only once on its
first execution and later re-use the allocated memory.

Before this patch the memory referenced by the pointers stored in the array
tmp_paths were released by the method Item_func_json_contains_path::cleanup
that is called on finishing execution of a prepared statement. Now that
memory allocation performed inside the method Item_json_str_multipath::fix_fields
is done only once, the item clean up has degenerate form and can be
delegated to the cleanup() method of the base class and memory deallocation
can be performed in the destructor.

mysql-test/main/ps_mem_leaks.result

@@ -45,4 +45,16 @@ a
 b
 DEALLOCATE PREPARE stmt;
 DROP TABLE t1,t2;
+#
+# MDEV-32733: Two JSON related tests running in PS mode fail on server
+#              built with -DWITH_PROTECT_STATEMENT_MEMROOT=YES
+#
+PREPARE stmt FROM "select json_contains_path('{\"key1\":1}', 'oNE', '$.key2[1]') as exp";
+EXECUTE stmt;
+exp
+0
+EXECUTE stmt;
+exp
+0
+DEALLOCATE PREPARE stmt;
 # End of 10.4 tests

mysql-test/main/ps_mem_leaks.test

@@ -58,4 +58,15 @@ DEALLOCATE PREPARE stmt;
 
 DROP TABLE t1,t2;
 
+--echo #
+--echo # MDEV-32733: Two JSON related tests running in PS mode fail on server
+--echo #              built with -DWITH_PROTECT_STATEMENT_MEMROOT=YES
+--echo #
+PREPARE stmt FROM "select json_contains_path('{\"key1\":1}', 'oNE', '$.key2[1]') as exp";
+
+EXECUTE stmt;
+EXECUTE stmt;
+
+DEALLOCATE PREPARE stmt;
+
 --echo # End of 10.4 tests

sql/item_jsonfunc.cc

@@ -859,21 +859,47 @@ static void mark_constant_paths(json_path_with_flags *p,
 }
 
 
-bool Item_json_str_multipath::fix_fields(THD *thd, Item **ref)
+Item_json_str_multipath::~Item_json_str_multipath()
 {
-  return alloc_tmp_paths(thd, get_n_paths(), &paths, &tmp_paths) ||
-         Item_str_func::fix_fields(thd, ref);
+  if (tmp_paths)
+  {
+    for (uint i= n_paths; i>0; i--)
+      tmp_paths[i-1].free();
+  }
 }
 
 
-void Item_json_str_multipath::cleanup()
+bool Item_json_str_multipath::fix_fields(THD *thd, Item **ref)
 {
-  if (tmp_paths)
+  if (!tmp_paths)
   {
-    for (uint i= get_n_paths(); i>0; i--)
-      tmp_paths[i-1].free();
+    /*
+      Remember the number of paths and allocate required memory on first time
+      the method fix_fields() is invoked. For prepared statements the method
+      fix_fields can be called several times for the same item because its
+      clean up is performed every item a prepared statement finishing its
+      execution. In result, the data member fixed is reset and the method
+      fix_field() is invoked on next time the same prepared statement be
+      executed. On the other side, any memory allocations on behalf of
+      the prepared statement must be performed only once on its first execution.
+      The data member tmp_path is kind a guard to do these activities only once
+      on first time the method fix_field() is called.
+    */
+    n_paths= get_n_paths();
+
+    if (alloc_tmp_paths(thd, n_paths, &paths, &tmp_paths))
+      return true;
   }
-  Item_str_func::cleanup();
+
+#ifdef PROTECT_STATEMENT_MEMROOT
+  /*
+   Check that the number of paths remembered on first run of a statement
+   never changed later.
+  */
+  DBUG_ASSERT(n_paths == get_n_paths());
+#endif
+
+  return Item_str_func::fix_fields(thd, ref);
 }
 
 
@@ -1404,10 +1430,19 @@ longlong Item_func_json_contains::val_int()
 
 bool Item_func_json_contains_path::fix_fields(THD *thd, Item **ref)
 {
-  return alloc_tmp_paths(thd, arg_count-2, &paths, &tmp_paths) ||
-         (p_found= (bool *) alloc_root(thd->mem_root,
-                                       (arg_count-2)*sizeof(bool))) == NULL ||
-         Item_int_func::fix_fields(thd, ref);
+  /*
+    See comments on Item_json_str_multipath::fix_fields regarding
+    the aim of the condition 'if (!tmp_paths)'.
+  */
+  if (!tmp_paths)
+  {
+    if (alloc_tmp_paths(thd, arg_count-2, &paths, &tmp_paths) ||
+        (p_found= (bool *) alloc_root(thd->mem_root,
+                                       (arg_count-2)*sizeof(bool))) == NULL)
+      return true;
+  }
+
+  return Item_int_func::fix_fields(thd, ref);
 }
 
 
@@ -1420,8 +1455,7 @@ bool Item_func_json_contains_path::fix_length_and_dec()
   return Item_bool_func::fix_length_and_dec();
 }
 
-
-void Item_func_json_contains_path::cleanup()
+Item_func_json_contains_path::~Item_func_json_contains_path()
 {
   if (tmp_paths)
   {
@@ -1429,7 +1463,6 @@ void Item_func_json_contains_path::cleanup()
       tmp_paths[i-1].free();
     tmp_paths= 0;
   }
-  Item_int_func::cleanup();
 }
 
 

sql/item_jsonfunc.h

@@ -144,11 +144,26 @@ class Item_json_str_multipath: public Item_str_func
 protected:
   json_path_with_flags *paths;
   String *tmp_paths;
+private:
+  /**
+    Number of paths returned by calling virtual method get_n_paths() and
+    remembered inside fix_fields(). It is used by the virtual destructor
+    ~Item_json_str_multipath() to iterate along allocated memory chunks stored
+    in the array tmp_paths and free every of them. The virtual method
+    get_n_paths() can't be used for this goal from within virtual destructor.
+    We could get rid of the virtual method get_n_paths() and store the number
+    of paths directly in the constructor of classes derived from the class
+    Item_json_str_multipath but presence of the method get_n_paths() allows
+    to check invariant that the number of arguments not changed between
+    sequential runs of the same prepared statement that seems to be useful.
+  */
+  uint n_paths;
 public:
   Item_json_str_multipath(THD *thd, List<Item> &list):
-    Item_str_func(thd, list), tmp_paths(0) {}
+    Item_str_func(thd, list), paths(NULL), tmp_paths(0), n_paths(0) {}
+  virtual ~Item_json_str_multipath();
+
   bool fix_fields(THD *thd, Item **ref);
-  void cleanup();
   virtual uint get_n_paths() const = 0;
   bool is_json_type() { return true; }
 };
@@ -208,10 +223,10 @@ class Item_func_json_contains_path: public Item_bool_func
 public:
   Item_func_json_contains_path(THD *thd, List<Item> &list):
     Item_bool_func(thd, list), tmp_paths(0) {}
+  virtual ~Item_func_json_contains_path();
   const char *func_name() const { return "json_contains_path"; }
   bool fix_fields(THD *thd, Item **ref);
   bool fix_length_and_dec();
-  void cleanup();
   longlong val_int();
   Item *get_copy(THD *thd)
   { return get_item_copy<Item_func_json_contains_path>(thd, this); }