An attempt to fix a sporadic valgrind memory leak in Event Scheduler:

streamline the event worker thread work flow and try to eliminate
possibilities for memory corruptions that might have been
lurking in previous (complicated) code.
This patch: 
 * removes Event_job_data::compile that was never used
 * cleans up Event_job_data::execute to minimize juggling with
   thread context and eliminate unneded code paths
 * Implements Security_context::change/restore_security_context
   to be able to re-use these methods in all stored programs
This is to maybe fix Bug#27733 "Valgrind failures in 
remove_table_from_cache".
Review comments applied.

sql/event_data_objects.cc

@@ -23,14 +23,6 @@
 
 #define EVEX_MAX_INTERVAL_VALUE 1000000000L
 
-static bool
-event_change_security_context(THD *thd, LEX_STRING user, LEX_STRING host,
-                              LEX_STRING db, Security_context *backup);
-
-static void
-event_restore_security_context(THD *thd, Security_context *backup);
-
-
 /*
   Initiliazes dbname and name of an Event_queue_element_for_exec
   object
@@ -816,27 +808,10 @@ Event_timed::~Event_timed()
 */
 
 Event_job_data::Event_job_data()
-  :sphead(NULL), sql_mode(0)
-{
-}
-
-
-/*
-  Destructor
-
-  SYNOPSIS
-    Event_timed::~Event_timed()
-*/
-
-Event_job_data::~Event_job_data()
+  :sql_mode(0)
 {
-  DBUG_ENTER("Event_job_data::~Event_job_data");
-  delete sphead;
-  sphead= NULL;
-  DBUG_VOID_RETURN;
 }
 
-
 /*
   Init all member variables
 
@@ -1769,234 +1744,202 @@ Event_timed::get_create_event(THD *thd, String *buf)
 }
 
 
-/*
-  Get SHOW CREATE EVENT as string
-
-  SYNOPSIS
-    Event_job_data::get_create_event(THD *thd, String *buf)
-      thd    Thread
-      buf    String*, should be already allocated. CREATE EVENT goes inside.
-
-  RETURN VALUE
-    0                       OK
-    EVEX_MICROSECOND_UNSUP  Error (for now if mysql.event has been
-                            tampered and MICROSECONDS interval or
-                            derivative has been put there.
+/**
+  Get an artificial stored procedure to parse as an event definition.
 */
 
-int
-Event_job_data::get_fake_create_event(String *buf)
+bool
+Event_job_data::construct_sp_sql(THD *thd, String *sp_sql)
 {
-  DBUG_ENTER("Event_job_data::get_create_event");
-  /* FIXME: "EVERY 3337 HOUR" is asking for trouble. */
-  buf->append(STRING_WITH_LEN("CREATE EVENT anonymous ON SCHEDULE "
-                              "EVERY 3337 HOUR DO "));
-  buf->append(body.str, body.length);
+  LEX_STRING buffer;
+  const uint STATIC_SQL_LENGTH= 44;
 
-  DBUG_RETURN(0);
+  DBUG_ENTER("Event_job_data::construct_sp_sql");
+
+  /*
+    Allocate a large enough buffer on the thread execution memory
+    root to avoid multiple [re]allocations on system heap
+  */
+  buffer.length= STATIC_SQL_LENGTH + name.length + body.length;
+  if (! (buffer.str= (char*) thd->alloc(buffer.length)))
+    DBUG_RETURN(TRUE);
+
+  sp_sql->set(buffer.str, buffer.length, system_charset_info);
+  sp_sql->length(0);
+
+
+  sp_sql->append(C_STRING_WITH_LEN("CREATE "));
+  sp_sql->append(C_STRING_WITH_LEN("PROCEDURE "));
+  /*
+    Let's use the same name as the event name to perhaps produce a
+    better error message in case it is a part of some parse error.
+    We're using append_identifier here to successfully parse
+    events with reserved names.
+  */
+  append_identifier(thd, sp_sql, name.str, name.length);
+
+  /*
+    The default SQL security of a stored procedure is DEFINER. We
+    have already activated the security context of the event, so
+    let's execute the procedure with the invoker rights to save on
+    resets of security contexts.
+  */
+  sp_sql->append(C_STRING_WITH_LEN("() SQL SECURITY INVOKER "));
+
+  sp_sql->append(body.str, body.length);
+
+  DBUG_RETURN(thd->is_fatal_error);
 }
 
 
-/*
-  Executes the event (the underlying sp_head object);
 
-  SYNOPSIS
-    Event_job_data::execute()
-      thd       THD
+/**
+  Compiles and executes the event (the underlying sp_head object)
 
-  RETURN VALUE
-    0        success
-    -99      No rights on this.dbname.str
-    others   retcodes of sp_head::execute_procedure()
+  @retval TRUE  error (reported to the error log)
+  @retval FALSE success
 */
 
-int
+bool
 Event_job_data::execute(THD *thd, bool drop)
 {
-  Security_context save_ctx;
-  /* this one is local and not needed after exec */
-  int ret= 0;
+  String sp_sql;
+  Security_context event_sctx, *save_sctx= NULL;
+  CHARSET_INFO *charset_connection;
+  List<Item> empty_item_list;
+  bool ret= TRUE;
 
   DBUG_ENTER("Event_job_data::execute");
-  DBUG_PRINT("info", ("EXECUTING %s.%s", dbname.str, name.str));
 
-  if ((ret= compile(thd, NULL)))
-    goto done;
+  mysql_reset_thd_for_next_command(thd);
 
-  event_change_security_context(thd, definer_user, definer_host, dbname,
-                                &save_ctx);
   /*
-    THD::~THD will clean this or if there is DROP DATABASE in the
-    SP then it will be free there. It should not point to our buffer
-    which is allocated on a mem_root.
+    MySQL parser currently assumes that current database is either
+    present in THD or all names in all statements are fully specified.
+    And yet not fully specified names inside stored programs must be 
+    be supported, even if the current database is not set:
+    CREATE PROCEDURE db1.p1() BEGIN CREATE TABLE t1; END//
+    -- in this example t1 should be always created in db1 and the statement
+    must parse even if there is no current database.
+
+    To support this feature and still address the parser limitation,
+    we need to set the current database here.
+    We don't have to call mysql_change_db, since the checks performed
+    in it are unnecessary for the purpose of parsing, and
+    mysql_change_db will be invoked anyway later, to activate the
+    procedure database before it's executed.
   */
-  thd->db= my_strdup(dbname.str, MYF(0));
-  thd->db_length= dbname.length;
-  if (!check_access(thd, EVENT_ACL,dbname.str, 0, 0, 0,is_schema_db(dbname.str)))
-  {
-    List<Item> empty_item_list;
-    empty_item_list.empty();
-    if (thd->enable_slow_log)
-      sphead->m_flags|= sp_head::LOG_SLOW_STATEMENTS;
-    sphead->m_flags|= sp_head::LOG_GENERAL_LOG;
-
-    /* Execute the event in its time zone. */
-    thd->variables.time_zone= time_zone;
+  thd->set_db(dbname.str, dbname.length);
 
-    ret= sphead->execute_procedure(thd, &empty_item_list);
-  }
-  else
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+  if (event_sctx.change_security_context(thd,
+                                         &definer_user, &definer_host,
+                                         &dbname, &save_sctx))
   {
-    DBUG_PRINT("error", ("%s@%s has no rights on %s", definer_user.str,
-               definer_host.str, dbname.str));
-    ret= -99;
+    sql_print_error("Event Scheduler: "
+                    "[%s].[%s.%s] execution failed, "
+                    "failed to authenticate the user.",
+                    definer.str, dbname.str, name.str);
+    goto end;
   }
-  if (drop)
+#endif
+
+  if (check_access(thd, EVENT_ACL, dbname.str,
+                   0, 0, 0, is_schema_db(dbname.str)))
   {
-    sql_print_information("Event Scheduler: Dropping %s.%s",
-                          dbname.str, name.str);
     /*
-      We must do it here since here we're under the right authentication
-      ID of the event definer
+      This aspect of behavior is defined in the worklog,
+      and this is how triggers work too: if TRIGGER
+      privilege is revoked from trigger definer,
+      triggers are not executed.
     */
-    if (Events::drop_event(thd, dbname, name, FALSE))
-      ret= 1;
+    sql_print_error("Event Scheduler: "
+                    "[%s].[%s.%s] execution failed, "
+                    "user no longer has EVENT privilege.",
+                    definer.str, dbname.str, name.str);
+    goto end;
   }
 
-  event_restore_security_context(thd, &save_ctx);
-done:
-  thd->end_statement();
-  thd->cleanup_after_query();
-
-  DBUG_PRINT("info", ("EXECUTED %s.%s  ret: %d", dbname.str, name.str, ret));
-
-  DBUG_RETURN(ret);
-}
-
-
-/*
-  Compiles an event before it's execution. Compiles the anonymous
-  sp_head object held by the event
+  if (construct_sp_sql(thd, &sp_sql))
+    goto end;
 
-  SYNOPSIS
-    Event_job_data::compile()
-      thd        thread context, used for memory allocation mostly
-      mem_root   if != NULL then this memory root is used for allocs
-                 instead of thd->mem_root
-
-  RETURN VALUE
-    0                       success
-    EVEX_COMPILE_ERROR      error during compilation
-    EVEX_MICROSECOND_UNSUP  mysql.event was tampered
-*/
-
-int
-Event_job_data::compile(THD *thd, MEM_ROOT *mem_root)
-{
-  int ret= 0;
-  MEM_ROOT *tmp_mem_root= 0;
-  LEX *old_lex= thd->lex, lex;
-  char *old_db;
-  int old_db_length;
-  char *old_query;
-  uint old_query_len;
-  ulong old_sql_mode= thd->variables.sql_mode;
-  char create_buf[15 * STRING_BUFFER_USUAL_SIZE];
-  String show_create(create_buf, sizeof(create_buf), system_charset_info);
-  CHARSET_INFO *old_character_set_client,
-               *old_collation_connection,
-               *old_character_set_results;
-  Security_context save_ctx;
-
-  DBUG_ENTER("Event_job_data::compile");
-
-  show_create.length(0);
-
-  switch (get_fake_create_event(&show_create)) {
-  case EVEX_MICROSECOND_UNSUP:
-    DBUG_RETURN(EVEX_MICROSECOND_UNSUP);
-  case 0:
-    break;
-  default:
-    DBUG_ASSERT(0);
-  }
+  /*
+    Set up global thread attributes to reflect the properties of
+    this Event. We can simply reset these instead of usual
+    backup/restore employed in stored programs since we know that
+    this is a top level statement and the worker thread is
+    allocated exclusively to execute this event.
+  */
+  charset_connection= get_charset_by_csname("utf8",
+                                            MY_CS_PRIMARY, MYF(MY_WME));
+  thd->variables.character_set_client= charset_connection;
+  thd->variables.character_set_results= charset_connection;
+  thd->variables.collation_connection= charset_connection;
+  thd->update_charset();
 
-  old_character_set_client= thd->variables.character_set_client;
-  old_character_set_results= thd->variables.character_set_results;
-  old_collation_connection= thd->variables.collation_connection;
+  thd->variables.sql_mode= sql_mode;
+  thd->variables.time_zone= time_zone;
 
-  thd->variables.character_set_client=
-    thd->variables.character_set_results=
-      thd->variables.collation_connection=
-           get_charset_by_csname("utf8", MY_CS_PRIMARY, MYF(MY_WME));
+  thd->query= sp_sql.c_ptr_safe();
+  thd->query_length= sp_sql.length();
 
-  thd->update_charset();
+  lex_start(thd, thd->query, thd->query_length);
 
-  DBUG_PRINT("info",("old_sql_mode: %lu  new_sql_mode: %lu",old_sql_mode, sql_mode));
-  thd->variables.sql_mode= this->sql_mode;
-  /* Change the memory root for the execution time */
-  if (mem_root)
+  if (MYSQLparse(thd) || thd->is_fatal_error)
   {
-    tmp_mem_root= thd->mem_root;
-    thd->mem_root= mem_root;
-  }
-  old_query_len= thd->query_length;
-  old_query= thd->query;
-  old_db= thd->db;
-  old_db_length= thd->db_length;
-  thd->db= dbname.str;
-  thd->db_length= dbname.length;
-
-  thd->query= show_create.c_ptr_safe();
-  thd->query_length= show_create.length();
-  DBUG_PRINT("info", ("query: %s",thd->query));
-
-  event_change_security_context(thd, definer_user, definer_host, dbname,
-                                &save_ctx);
-  thd->lex= &lex;
-  mysql_init_query(thd, thd->query, thd->query_length);
-  if (MYSQLparse((void *)thd) || thd->is_fatal_error)
-  {
-    DBUG_PRINT("error", ("error during compile or thd->is_fatal_error: %d",
-                          thd->is_fatal_error));
-    lex.unit.cleanup();
-
     sql_print_error("Event Scheduler: "
                     "%serror during compilation of %s.%s",
                     thd->is_fatal_error ? "fatal " : "",
-                    dbname.str, name.str);
-
-    ret= EVEX_COMPILE_ERROR;
-    goto done;
+                    (const char *) dbname.str, (const char *) name.str);
+    goto end;
   }
-  DBUG_PRINT("note", ("success compiling %s.%s", dbname.str, name.str));
 
-  sphead= lex.sphead;
+  {
+    sp_head *sphead= thd->lex->sphead;
 
-  sphead->set_definer(definer.str, definer.length);
-  sphead->set_info(0, 0, &lex.sp_chistics, sql_mode);
-  sphead->optimize();
-  ret= 0;
-done:
+    DBUG_ASSERT(sphead);
 
-  lex_end(&lex);
-  event_restore_security_context(thd, &save_ctx);
-  DBUG_PRINT("note", ("return old data on its place. set back NAMES"));
+    if (thd->enable_slow_log)
+      sphead->m_flags|= sp_head::LOG_SLOW_STATEMENTS;
+    sphead->m_flags|= sp_head::LOG_GENERAL_LOG;
 
-  thd->lex= old_lex;
-  thd->query= old_query;
-  thd->query_length= old_query_len;
-  thd->db= old_db;
+    sphead->set_info(0, 0, &thd->lex->sp_chistics, sql_mode);
+    sphead->optimize();
 
-  thd->variables.sql_mode= old_sql_mode;
-  thd->variables.character_set_client= old_character_set_client;
-  thd->variables.character_set_results= old_character_set_results;
-  thd->variables.collation_connection= old_collation_connection;
-  thd->update_charset();
+    ret= sphead->execute_procedure(thd, &empty_item_list);
+    /*
+      There is no pre-locking and therefore there should be no
+      tables open and locked left after execute_procedure.
+    */
+  }
 
-  /* Change the memory root for the execution time. */
-  if (mem_root)
-    thd->mem_root= tmp_mem_root;
+end:
+  if (drop && !thd->is_fatal_error)
+  {
+    sql_print_information("Event Scheduler: Dropping %s.%s",
+                          (const char *) dbname.str, (const char *) name.str);
+    /*
+      We must do it here since here we're under the right authentication
+      ID of the event definer.
+    */
+    if (Events::drop_event(thd, dbname, name, FALSE))
+      ret= 1;
+  }
+  if (thd->lex->sphead)                        /* NULL only if a parse error */
+  {
+    delete thd->lex->sphead;
+    thd->lex->sphead= NULL;
+  }
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+  if (save_sctx)
+    event_sctx.restore_security_context(thd, save_sctx);
+#endif
+  lex_end(thd->lex);
+  thd->lex->unit.cleanup();
+  thd->end_statement();
+  thd->cleanup_after_query();
+
+  DBUG_PRINT("info", ("EXECUTED %s.%s  ret: %d", dbname.str, name.str, ret));
 
   DBUG_RETURN(ret);
 }
@@ -2042,64 +1985,3 @@ event_basic_identifier_equal(LEX_STRING db, LEX_STRING name, Event_basic *b)
   return !sortcmp_lex_string(name, b->name, system_charset_info) &&
          !sortcmp_lex_string(db, b->dbname, system_charset_info);
 }
-
-
-/*
-  Switches the security context.
-
-  SYNOPSIS
-    event_change_security_context()
-      thd     Thread
-      user    The user
-      host    The host of the user
-      db      The schema for which the security_ctx will be loaded
-      backup  Where to store the old context
-
-  RETURN VALUE
-    FALSE  OK
-    TRUE   Error (generates error too)
-*/
-
-static bool
-event_change_security_context(THD *thd, LEX_STRING user, LEX_STRING host,
-                              LEX_STRING db, Security_context *backup)
-{
-  DBUG_ENTER("event_change_security_context");
-  DBUG_PRINT("info",("%s@%s@%s", user.str, host.str, db.str));
-#ifndef NO_EMBEDDED_ACCESS_CHECKS
-
-  *backup= thd->main_security_ctx;
-  if (acl_getroot_no_password(&thd->main_security_ctx, user.str, host.str,
-                              host.str, db.str))
-  {
-    my_error(ER_NO_SUCH_USER, MYF(0), user.str, host.str);
-    DBUG_RETURN(TRUE);
-  }
-  thd->security_ctx= &thd->main_security_ctx;
-#endif
-  DBUG_RETURN(FALSE);
-}
-
-
-/*
-  Restores the security context.
-
-  SYNOPSIS
-    event_restore_security_context()
-      thd     Thread
-      backup  Context to switch to
-*/
-
-static void
-event_restore_security_context(THD *thd, Security_context *backup)
-{
-  DBUG_ENTER("event_restore_security_context");
-#ifndef NO_EMBEDDED_ACCESS_CHECKS
-  if (backup)
-  {
-    thd->main_security_ctx= *backup;
-    thd->security_ctx= &thd->main_security_ctx;
-  }
-#endif
-  DBUG_VOID_RETURN;
-}

sql/event_data_objects.h

@@ -17,7 +17,6 @@
 
 
 #define EVEX_GET_FIELD_FAILED   -2
-#define EVEX_COMPILE_ERROR      -3
 #define EVEX_BAD_PARAMS         -5
 #define EVEX_MICROSECOND_UNSUP  -6
 
@@ -169,8 +168,6 @@ class Event_timed : public Event_queue_element
 class Event_job_data : public Event_basic
 {
 public:
-  sp_head *sphead;
-
   LEX_STRING body;
   LEX_STRING definer_user;
   LEX_STRING definer_host;
@@ -178,19 +175,15 @@ class Event_job_data : public Event_basic
   ulong sql_mode;
 
   Event_job_data();
-  virtual ~Event_job_data();
 
   virtual int
   load_from_row(THD *thd, TABLE *table);
 
-  int
+  bool
   execute(THD *thd, bool drop);
-
-  int
-  compile(THD *thd, MEM_ROOT *mem_root);
 private:
-  int
-  get_fake_create_event(String *buf);
+  bool
+  construct_sp_sql(THD *thd, String *sp_sql);
 
   Event_job_data(const Event_job_data &);       /* Prevent use of these */
   void operator=(Event_job_data &);

sql/event_scheduler.cc

@@ -277,8 +277,7 @@ Event_worker_thread::run(THD *thd, Event_queue_element_for_exec *event)
 {
   /* needs to be first for thread_stack */
   char my_stack;
-  int ret;
-  Event_job_data *job_data= NULL;
+  Event_job_data job_data;
   bool res;
 
   thd->thread_stack= &my_stack;                // remember where our stack is
@@ -291,60 +290,43 @@ Event_worker_thread::run(THD *thd, Event_queue_element_for_exec *event)
   if (res)
     goto end;
 
-  if (!(job_data= new Event_job_data()))
-    goto end;
-  else if ((ret= db_repository->
-                  load_named_event(thd, event->dbname, event->name, job_data)))
+  if ((res= db_repository->load_named_event(thd, event->dbname, event->name,
+                                            &job_data)))
   {
-    DBUG_PRINT("error", ("Got %d from load_named_event", ret));
+    DBUG_PRINT("error", ("Got error from load_named_event"));
     goto end;
   }
 
   sql_print_information("Event Scheduler: "
-                        "[%s.%s of %s] executing in thread %lu. ",
-                        job_data->dbname.str, job_data->name.str,
-                        job_data->definer.str, thd->thread_id);
+                        "[%s].[%s.%s] started in thread %lu.",
+                        job_data.definer.str,
+                        job_data.dbname.str, job_data.name.str,
+                        thd->thread_id);
 
   thd->enable_slow_log= TRUE;
 
-  ret= job_data->execute(thd, event->dropped);
+  res= job_data.execute(thd, event->dropped);
 
-  print_warnings(thd, job_data);
+  print_warnings(thd, &job_data);
 
-  switch (ret) {
-  case 0:
+  if (res)
+    sql_print_information("Event Scheduler: "
+                          "[%s].[%s.%s] event execution failed.",
+                          job_data.definer.str,
+                          job_data.dbname.str, job_data.name.str);
+  else
     sql_print_information("Event Scheduler: "
                           "[%s].[%s.%s] executed successfully in thread %lu.",
-                          job_data->definer.str,
-                          job_data->dbname.str, job_data->name.str,
+                          job_data.definer.str,
+                          job_data.dbname.str, job_data.name.str,
                           thd->thread_id);
-    break;
-  case EVEX_COMPILE_ERROR:
-    sql_print_information("Event Scheduler: "
-                          "[%s].[%s.%s] event compilation failed.",
-                          job_data->definer.str,
-                          job_data->dbname.str, job_data->name.str);
-    break;
-  default:
-    sql_print_information("Event Scheduler: "
-                          "[%s].[%s.%s] event execution failed.",
-                          job_data->definer.str,
-                          job_data->dbname.str, job_data->name.str);
-    break;
-  }
 
 end:
-  delete job_data;
-
   DBUG_PRINT("info", ("Done with Event %s.%s", event->dbname.str,
              event->name.str));
 
   delete event;
   deinit_event_thread(thd);
-  /*
-    Do not pthread_exit since we want local destructors for stack objects
-    to be invoked.
-  */
 }
 
 

sql/item_func.cc

@@ -5351,7 +5351,7 @@ Item_func_sp::fix_fields(THD *thd, Item **ref)
     Security_context *save_secutiry_ctx;
     res= set_routine_security_ctx(thd, m_sp, false, &save_secutiry_ctx);
     if (!res)
-      sp_restore_security_context(thd, save_secutiry_ctx);
+      m_sp->m_security_ctx.restore_security_context(thd, save_secutiry_ctx);
     
 #endif /* ! NO_EMBEDDED_ACCESS_CHECKS */
   }

sql/sp_head.cc

@@ -1232,7 +1232,11 @@ set_routine_security_ctx(THD *thd, sp_head *sp, bool is_proc,
                          Security_context **save_ctx)
 {
   *save_ctx= 0;
-  if (sp_change_security_context(thd, sp, save_ctx))
+  if (sp->m_chistics->suid != SP_IS_NOT_SUID &&
+      sp->m_security_ctx.change_security_context(thd, &sp->m_definer_user,
+                                                 &sp->m_definer_host,
+                                                 &sp->m_db,
+                                                 save_ctx))
     return TRUE;
 
   /*
@@ -1249,7 +1253,7 @@ set_routine_security_ctx(THD *thd, sp_head *sp, bool is_proc,
       check_routine_access(thd, EXECUTE_ACL,
                            sp->m_db.str, sp->m_name.str, is_proc, FALSE))
   {
-    sp_restore_security_context(thd, *save_ctx);
+    sp->m_security_ctx.restore_security_context(thd, *save_ctx);
     *save_ctx= 0;
     return TRUE;
   }
@@ -1560,7 +1564,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount,
   }
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
-  sp_restore_security_context(thd, save_security_ctx);
+  m_security_ctx.restore_security_context(thd, save_security_ctx);
 #endif
 
 err_with_cleanup:
@@ -1778,7 +1782,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
   if (save_security_ctx)
-    sp_restore_security_context(thd, save_security_ctx);
+    m_security_ctx.restore_security_context(thd, save_security_ctx);
 #endif
 
   if (!save_spcont)
@@ -3418,44 +3422,6 @@ sp_instr_set_case_expr::opt_move(uint dst, List<sp_instr> *bp)
 
 /* ------------------------------------------------------------------ */
 
-/*
-  Security context swapping
-*/
-
-#ifndef NO_EMBEDDED_ACCESS_CHECKS
-bool
-sp_change_security_context(THD *thd, sp_head *sp, Security_context **backup)
-{
-  *backup= 0;
-  if (sp->m_chistics->suid != SP_IS_NOT_SUID &&
-      (strcmp(sp->m_definer_user.str,
-              thd->security_ctx->priv_user) ||
-       my_strcasecmp(system_charset_info, sp->m_definer_host.str,
-                     thd->security_ctx->priv_host)))
-  {
-    if (acl_getroot_no_password(&sp->m_security_ctx, sp->m_definer_user.str,
-                                sp->m_definer_host.str,
-                                sp->m_definer_host.str,
-                                sp->m_db.str))
-    {
-      my_error(ER_NO_SUCH_USER, MYF(0), sp->m_definer_user.str,
-               sp->m_definer_host.str);
-      return TRUE;
-    }
-    *backup= thd->security_ctx;
-    thd->security_ctx= &sp->m_security_ctx;
-  }
-  return FALSE;
-}
-
-void
-sp_restore_security_context(THD *thd, Security_context *backup)
-{
-  if (backup)
-    thd->security_ctx= backup;
-}
-
-#endif /* NO_EMBEDDED_ACCESS_CHECKS */
 
 /*
   Structure that represent all instances of one table

sql/sql_class.cc

@@ -2119,6 +2119,102 @@ bool Security_context::set_user(char *user_arg)
   return user == 0;
 }
 
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+/**
+  Initialize this security context from the passed in credentials
+  and activate it in the current thread.
+
+  @param[out]  backup  Save a pointer to the current security context
+                       in the thread. In case of success it points to the
+                       saved old context, otherwise it points to NULL.
+
+
+  During execution of a statement, multiple security contexts may
+  be needed:
+  - the security context of the authenticated user, used as the
+    default security context for all top-level statements
+  - in case of a view or a stored program, possibly the security
+    context of the definer of the routine, if the object is
+    defined with SQL SECURITY DEFINER option.
+
+  The currently "active" security context is parameterized in THD
+  member security_ctx. By default, after a connection is
+  established, this member points at the "main" security context
+  - the credentials of the authenticated user.
+
+  Later, if we would like to execute some sub-statement or a part
+  of a statement under credentials of a different user, e.g.
+  definer of a procedure, we authenticate this user in a local
+  instance of Security_context by means of this method (and
+  ultimately by means of acl_getroot_no_password), and make the
+  local instance active in the thread by re-setting
+  thd->security_ctx pointer.
+
+  Note, that the life cycle and memory management of the "main" and
+  temporary security contexts are different.
+  For the main security context, the memory for user/host/ip is
+  allocated on system heap, and the THD class frees this memory in
+  its destructor. The only case when contents of the main security
+  context may change during its life time is when someone issued
+  CHANGE USER command.
+  Memory management of a "temporary" security context is
+  responsibility of the module that creates it.
+
+  @retval TRUE  there is no user with the given credentials. The erro
+                is reported in the thread.
+  @retval FALSE success
+*/
+
+bool
+Security_context::
+change_security_context(THD *thd,
+                        LEX_STRING *definer_user,
+                        LEX_STRING *definer_host,
+                        LEX_STRING *db,
+                        Security_context **backup)
+{
+  bool needs_change;
+
+  DBUG_ENTER("Security_context::change_security_context");
+
+  DBUG_ASSERT(definer_user->str && definer_host->str);
+
+  *backup= NULL;
+  /*
+    The current security context may have NULL members
+    if we have just started the thread and not authenticated
+    any user. This use case is currently in events worker thread.
+  */
+  needs_change= (thd->security_ctx->priv_user == NULL ||
+                 strcmp(definer_user->str, thd->security_ctx->priv_user) ||
+                 thd->security_ctx->priv_host == NULL ||
+                 my_strcasecmp(system_charset_info, definer_host->str,
+                               thd->security_ctx->priv_host));
+  if (needs_change)
+  {
+    if (acl_getroot_no_password(this, definer_user->str, definer_host->str,
+                                definer_host->str, db->str))
+    {
+      my_error(ER_NO_SUCH_USER, MYF(0), definer_user->str,
+               definer_host->str);
+      DBUG_RETURN(TRUE);
+    }
+    *backup= thd->security_ctx;
+    thd->security_ctx= this;
+  }
+
+  DBUG_RETURN(FALSE);
+}
+
+
+void
+Security_context::restore_security_context(THD *thd,
+                                           Security_context *backup)
+{
+  if (backup)
+    thd->security_ctx= backup;
+}
+#endif
 
 /****************************************************************************
   Handling of open and locked tables states.

sql/sql_class.h

@@ -656,6 +656,18 @@ class Security_context {
   }
   
   bool set_user(char *user_arg);
+
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+  bool
+  change_security_context(THD *thd,
+                          LEX_STRING *definer_user,
+                          LEX_STRING *definer_host,
+                          LEX_STRING *db,
+                          Security_context **backup);
+
+  void
+  restore_security_context(THD *thd, Security_context *backup);
+#endif
 };
 
 

sql/sql_trigger.cc

@@ -1543,9 +1543,15 @@ bool Table_triggers_list::process_triggers(THD *thd, trg_event_type event,
       old_field= trigger_table->field;
     }
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
+    Security_context *sctx= &sp_trigger->m_security_ctx;
     Security_context *save_ctx;
 
-    if (sp_change_security_context(thd, sp_trigger, &save_ctx))
+
+    if (sctx->change_security_context(thd,
+                                      &sp_trigger->m_definer_user,
+                                      &sp_trigger->m_definer_host,
+                                      &sp_trigger->m_db,
+                                      &save_ctx))
       return TRUE;
 
     /*
@@ -1570,7 +1576,7 @@ bool Table_triggers_list::process_triggers(THD *thd, trg_event_type event,
                thd->security_ctx->priv_user, thd->security_ctx->host_or_ip,
                trigger_table->s->table_name.str);
 
-      sp_restore_security_context(thd, save_ctx);
+      sctx->restore_security_context(thd, save_ctx);
       return TRUE;
     }
 #endif // NO_EMBEDDED_ACCESS_CHECKS
@@ -1582,7 +1588,7 @@ bool Table_triggers_list::process_triggers(THD *thd, trg_event_type event,
     thd->restore_sub_statement_state(&statement_state);
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
-    sp_restore_security_context(thd, save_ctx);
+    sctx->restore_security_context(thd, save_ctx);
 #endif // NO_EMBEDDED_ACCESS_CHECKS
   }