MDEV-16584 SP with a cursor inside a loop wastes THD memory aggressively

Problem:

push_handler() created sp_handler_entry instances on THD::main_mem_root,
which is freed only after the SP instructions execution.
So in case of a CONTINUE HANDLER inside a loop (e.g. WHILE) this approach
leaked thread memory on every loop iteration.

Changes:
- Removing sp_handler_entry declaration, it's not really needed.
- Fixing the data type of sp_rcontext::m_handlers from
  Dynamic_array<sp_handler_entry*> to Dynamic_array<sp_instr_hpush_jump*>
- Fixing sp_rcontext::push_handler() to push the pointer to
  an sp_instr_hpush_jump instance to the handler stack.
  This instance contains everything we need.
  There is no a need to allocate anything else.

mysql-test/main/sp-condition-handler.result

+#
+# Start of 10.3 tests
+#
+#
+# MDEV-16595 SP with a CONTINUE HANDLER inside a loop wastes THD memory aggressively
+#
+CREATE PROCEDURE p1()
+BEGIN
+DECLARE mem_used_old BIGINT UNSIGNED DEFAULT
+(SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.SESSION_STATUS
+WHERE VARIABLE_NAME='MEMORY_USED');
+DECLARE i INT DEFAULT 1;
+WHILE i <= 1000
+DO
+BEGIN
+DECLARE msg TEXT;
+DECLARE mem_used_cur BIGINT UNSIGNED DEFAULT
+(SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.SESSION_STATUS
+WHERE VARIABLE_NAME='MEMORY_USED');
+DECLARE CONTINUE HANDLER FOR SQLSTATE '23000' SET @x=1;
+DECLARE CONTINUE HANDLER FOR SQLSTATE '23001' SET @x=1;
+DECLARE CONTINUE HANDLER FOR SQLSTATE '23002' SET @x=1;
+DECLARE CONTINUE HANDLER FOR SQLSTATE '23003' SET @x=1;
+DECLARE CONTINUE HANDLER FOR SQLSTATE '23004' SET @x=1;
+DECLARE CONTINUE HANDLER FOR SQLSTATE '23005' SET @x=1;
+DECLARE CONTINUE HANDLER FOR SQLSTATE '23006' SET @x=1;
+DECLARE CONTINUE HANDLER FOR SQLSTATE '23007' SET @x=1;
+DECLARE CONTINUE HANDLER FOR SQLSTATE '23008' SET @x=1;
+IF (mem_used_cur >= mem_used_old * 1.1) THEN
+SHOW STATUS LIKE 'Memory_used';
+SET msg=CONCAT('Memory leak detected: i=', i, ' mem_used_old=',mem_used_old,' mem_used_cur=', mem_used_cur);
+SIGNAL SQLSTATE '45000' SET MESSAGE_TEXT=msg;
+END IF;
+END;
+SET i=i+1;
+END WHILE;
+END;
+$$
+CALL p1;
+DROP PROCEDURE p1;
+#
+# End of 10.3 tests
+#

mysql-test/main/sp-condition-handler.test

+
+--echo #
+--echo # Start of 10.3 tests
+--echo #
+
+--echo #
+--echo # MDEV-16595 SP with a CONTINUE HANDLER inside a loop wastes THD memory aggressively
+--echo #
+
+DELIMITER $$;
+CREATE PROCEDURE p1()
+BEGIN
+  DECLARE mem_used_old BIGINT UNSIGNED DEFAULT
+    (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.SESSION_STATUS
+     WHERE VARIABLE_NAME='MEMORY_USED');
+  DECLARE i INT DEFAULT 1;
+  WHILE i <= 1000
+  DO
+    BEGIN
+      DECLARE msg TEXT;
+      DECLARE mem_used_cur BIGINT UNSIGNED DEFAULT
+        (SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.SESSION_STATUS
+         WHERE VARIABLE_NAME='MEMORY_USED');
+      DECLARE CONTINUE HANDLER FOR SQLSTATE '23000' SET @x=1;
+      DECLARE CONTINUE HANDLER FOR SQLSTATE '23001' SET @x=1;
+      DECLARE CONTINUE HANDLER FOR SQLSTATE '23002' SET @x=1;
+      DECLARE CONTINUE HANDLER FOR SQLSTATE '23003' SET @x=1;
+      DECLARE CONTINUE HANDLER FOR SQLSTATE '23004' SET @x=1;
+      DECLARE CONTINUE HANDLER FOR SQLSTATE '23005' SET @x=1;
+      DECLARE CONTINUE HANDLER FOR SQLSTATE '23006' SET @x=1;
+      DECLARE CONTINUE HANDLER FOR SQLSTATE '23007' SET @x=1;
+      DECLARE CONTINUE HANDLER FOR SQLSTATE '23008' SET @x=1;
+      IF (mem_used_cur >= mem_used_old * 1.1) THEN
+        SHOW STATUS LIKE 'Memory_used';
+        SET msg=CONCAT('Memory leak detected: i=', i, ' mem_used_old=',mem_used_old,' mem_used_cur=', mem_used_cur);
+        SIGNAL SQLSTATE '45000' SET MESSAGE_TEXT=msg;
+      END IF;
+    END;
+    SET i=i+1;
+  END WHILE;
+END;
+$$
+DELIMITER ;$$
+CALL p1;
+DROP PROCEDURE p1;
+
+
+--echo #
+--echo # End of 10.3 tests
+--echo #

sql/sp_head.cc

@@ -4027,7 +4027,7 @@ sp_instr_hpush_jump::execute(THD *thd, uint *nextp)
 {
   DBUG_ENTER("sp_instr_hpush_jump::execute");
 
-  int ret= thd->spcont->push_handler(m_handler, m_ip + 1);
+  int ret= thd->spcont->push_handler(this);
 
   *nextp= m_dest;
 

sql/sp_head.h

@@ -1676,8 +1676,6 @@ class sp_instr_hpush_jump : public sp_instr_jump
   sp_handler *get_handler()
   { return m_handler; }
 
-private:
-
 private:
   /// Handler.
   sp_handler *m_handler;

sql/sp_rcontext.cc

@@ -448,19 +448,9 @@ void sp_rcontext::pop_cursors(THD *thd, size_t count)
 }
 
 
-bool sp_rcontext::push_handler(sp_handler *handler, uint first_ip)
+bool sp_rcontext::push_handler(sp_instr_hpush_jump *entry)
 {
-  /*
-    We should create handler entries in the callers arena, as
-    they could be (and usually are) used in several instructions.
-  */
-  sp_handler_entry *he=
-    new (callers_arena->mem_root) sp_handler_entry(handler, first_ip);
-
-  if (he == NULL)
-    return true;
-
-  return m_handlers.append(he);
+  return m_handlers.append(entry);
 }
 
 
@@ -548,12 +538,12 @@ bool sp_rcontext::handle_sql_condition(THD *thd,
 
   DBUG_ASSERT(found_condition);
 
-  sp_handler_entry *handler_entry= NULL;
+  sp_instr_hpush_jump *handler_entry= NULL;
   for (size_t i= 0; i < m_handlers.elements(); ++i)
   {
-    sp_handler_entry *h= m_handlers.at(i);
+    sp_instr_hpush_jump *h= m_handlers.at(i);
 
-    if (h->handler == found_handler)
+    if (h->get_handler() == found_handler)
     {
       handler_entry= h;
       break;
@@ -582,7 +572,7 @@ bool sp_rcontext::handle_sql_condition(THD *thd,
   // Mark active conditions so that they can be deleted when the handler exits.
   da->mark_sql_conditions_for_removal();
 
-  uint continue_ip= handler_entry->handler->type == sp_handler::CONTINUE ?
+  uint continue_ip= handler_entry->get_handler()->type == sp_handler::CONTINUE ?
     cur_spi->get_cont_dest() : 0;
 
   /* End aborted result set. */
@@ -601,7 +591,7 @@ bool sp_rcontext::handle_sql_condition(THD *thd,
     new (callers_arena->mem_root) Handler_call_frame(cond_info, continue_ip);
   m_handler_call_stack.append(frame);
 
-  *ip= handler_entry->first_ip;
+  *ip= handler_entry->m_ip + 1;
 
   DBUG_RETURN(true);
 }

sql/sp_rcontext.h

@@ -31,6 +31,7 @@
 class sp_cursor;
 class sp_lex_keeper;
 class sp_instr_cpush;
+class sp_instr_hpush_jump;
 class Query_arena;
 class sp_head;
 class Item_cache;
@@ -87,27 +88,6 @@ class sp_rcontext : public Sql_alloc
   sp_rcontext(const sp_rcontext &);
   void operator=(sp_rcontext &);
 
-private:
-  /// This is an auxillary class to store entering instruction pointer for an
-  /// SQL-handler.
-  class sp_handler_entry : public Sql_alloc
-  {
-  public:
-    /// Handler definition (from parsing context).
-    const sp_handler *handler;
-
-    /// Instruction pointer to the first instruction.
-    uint first_ip;
-
-    /// The constructor.
-    ///
-    /// @param _handler   sp_handler object.
-    /// @param _first_ip  first instruction pointer.
-    sp_handler_entry(const sp_handler *_handler, uint _first_ip)
-     :handler(_handler), first_ip(_first_ip)
-    { }
-  };
-
 public:
   /// This class stores basic information about SQL-condition, such as:
   ///   - SQL error code;
@@ -235,18 +215,16 @@ class sp_rcontext : public Sql_alloc
   // SQL-handlers.
   /////////////////////////////////////////////////////////////////////////
 
-  /// Create a new sp_handler_entry instance and push it to the handler call
-  /// stack.
+  /// Push an sp_instr_hpush_jump instance to the handler call stack.
   ///
-  /// @param handler  SQL-handler object.
-  /// @param first_ip First instruction pointer of the handler.
+  /// @param entry    The condition handler entry
   ///
   /// @return error flag.
   /// @retval false on success.
   /// @retval true on error.
-  bool push_handler(sp_handler *handler, uint first_ip);
+  bool push_handler(sp_instr_hpush_jump *entry);
 
-  /// Pop and delete given number of sp_handler_entry instances from the handler
+  /// Pop and delete given number of instances from the handler
   /// call stack.
   ///
   /// @param count Number of handler entries to pop & delete.
@@ -411,7 +389,7 @@ class sp_rcontext : public Sql_alloc
   bool m_in_sub_stmt;
 
   /// Stack of visible handlers.
-  Dynamic_array<sp_handler_entry *> m_handlers;
+  Dynamic_array<sp_instr_hpush_jump *> m_handlers;
 
   /// Stack of caught SQL conditions.
   Dynamic_array<Handler_call_frame *> m_handler_call_stack;