Bug#43587: Putting event_scheduler=1 in init SQL file crashes

mysqld

The problem was that enabling the event scheduler inside a init
file caused the server to crash upon start-up. The crash occurred
because the event scheduler wasn't being initialized before the
commands in the init-file are processed.

The solution is to initialize the event scheduler before the init
file is read. The patch also disables the event scheduler during
bootstrap and makes the bootstrap operation robust in the
presence of background threads.

mysql-test/std_data/init_file.dat

@@ -35,4 +35,11 @@ CREATE DATABASE IF NOT EXISTS init_file;
 CREATE TABLE IF NOT EXISTS init_file.startup ( startdate DATETIME );
 INSERT INTO init_file.startup VALUES ( NOW() );
 
+#
+# Bug#43587 "Putting event_scheduler=1 in init SQL file crashes mysqld"
+#
 
+SET GLOBAL event_scheduler = 'ON';
+CREATE EVENT ev1 ON SCHEDULE EVERY 1 DAY DISABLE DO SELECT 1;
+DROP EVENT ev1;
+SET GLOBAL event_scheduler = 'OFF';

sql/event_scheduler.cc

@@ -158,6 +158,7 @@ deinit_event_thread(THD *thd)
   thread_count--;
   thread_running--;
   delete thd;
+  pthread_cond_broadcast(&COND_thread_count);
   pthread_mutex_unlock(&LOCK_thread_count);
 }
 
@@ -418,6 +419,7 @@ Event_scheduler::start()
     thread_count--;
     thread_running--;
     delete new_thd;
+    pthread_cond_broadcast(&COND_thread_count);
     pthread_mutex_unlock(&LOCK_thread_count);
   }
 end:
@@ -550,6 +552,7 @@ error:
     thread_count--;
     thread_running--;
     delete new_thd;
+    pthread_cond_broadcast(&COND_thread_count);
     pthread_mutex_unlock(&LOCK_thread_count);
   }
   delete event_name;

sql/events.cc

@@ -852,22 +852,23 @@ Events::fill_schema_events(THD *thd, TABLE_LIST *tables, COND * /* cond */)
 }
 
 
-/*
-  Inits the scheduler's structures.
+/**
+  Initializes the scheduler's structures.
 
-  SYNOPSIS
-    Events::init()
+  @param  opt_noacl_or_bootstrap
+                     TRUE if there is --skip-grant-tables or --bootstrap
+                     option. In that case we disable the event scheduler.
 
-  NOTES
-    This function is not synchronized.
+  @note   This function is not synchronized.
 
-  RETURN VALUE
-    FALSE  OK
-    TRUE   Error in case the scheduler can't start
+  @retval  FALSE   Perhaps there was an error, and the event scheduler
+                   is disabled. But the error is not fatal and the 
+                   server start up can continue.
+  @retval  TRUE    Fatal error. Startup must terminate (call unireg_abort()).
 */
 
 bool
-Events::init(my_bool opt_noacl)
+Events::init(my_bool opt_noacl_or_bootstrap)
 {
 
   THD *thd;
@@ -875,11 +876,6 @@ Events::init(my_bool opt_noacl)
 
   DBUG_ENTER("Events::init");
 
-  /* Disable the scheduler if running with --skip-grant-tables */
-  if (opt_noacl)
-    opt_event_scheduler= EVENTS_DISABLED;
-
-
   /* We need a temporary THD during boot */
   if (!(thd= new THD()))
   {
@@ -908,23 +904,30 @@ Events::init(my_bool opt_noacl)
   /*
     Since we allow event DDL even if the scheduler is disabled,
     check the system tables, as we might need them.
+
+    If run with --skip-grant-tables or --bootstrap, don't try to do the
+    check of system tables and don't complain: in these modes the tables
+    are most likely not there and we're going to disable the event
+    scheduler anyway.
   */
-  if (Event_db_repository::check_system_tables(thd))
+  if (opt_noacl_or_bootstrap || Event_db_repository::check_system_tables(thd))
+  {
+    if (! opt_noacl_or_bootstrap)
     {
       sql_print_error("Event Scheduler: An error occurred when initializing "
-                    "system tables.%s",
-                    opt_event_scheduler == EVENTS_DISABLED ?
-                    "" : " Disabling the Event Scheduler.");
+                      "system tables. Disabling the Event Scheduler.");
+      check_system_tables_error= TRUE;
+    }
 
     /* Disable the scheduler since the system tables are not up to date */
     opt_event_scheduler= EVENTS_DISABLED;
-    check_system_tables_error= TRUE;
     goto end;
   }
 
   /*
     Was disabled explicitly from the command line, or because we're running
-    with --skip-grant-tables, or because we have no system tables.
+    with --skip-grant-tables, or --bootstrap, or because we have no system
+    tables.
   */
   if (opt_event_scheduler == Events::EVENTS_DISABLED)
     goto end;
@@ -941,7 +944,7 @@ Events::init(my_bool opt_noacl)
   }
 
   if (event_queue->init_queue(thd) || load_events_from_db(thd) ||
-      opt_event_scheduler == EVENTS_ON && scheduler->start())
+      (opt_event_scheduler == EVENTS_ON && scheduler->start()))
   {
     sql_print_error("Event Scheduler: Error while loading from disk.");
     res= TRUE; /* fatal error: request unireg_abort */

sql/mysql_priv.h

@@ -1976,6 +1976,7 @@ extern bool opt_disable_networking, opt_skip_show_db;
 extern bool opt_ignore_builtin_innodb;
 extern my_bool opt_character_set_client_handshake;
 extern bool volatile abort_loop, shutdown_in_progress;
+extern bool in_bootstrap;
 extern uint volatile thread_count, thread_running, global_read_lock;
 extern uint connection_count;
 extern my_bool opt_sql_bin_update, opt_safe_user_create, opt_no_mix_types;

sql/mysqld.cc

@@ -416,6 +416,21 @@ my_bool locked_in_memory;
 bool opt_using_transactions;
 bool volatile abort_loop;
 bool volatile shutdown_in_progress;
+/*
+  True if the bootstrap thread is running. Protected by LOCK_thread_count,
+  just like thread_count.
+  Used in bootstrap() function to determine if the bootstrap thread
+  has completed. Note, that we can't use 'thread_count' instead,
+  since in 5.1, in presence of the Event Scheduler, there may be
+  event threads running in parallel, so it's impossible to know
+  what value of 'thread_count' is a sign of completion of the
+  bootstrap thread.
+
+  At the same time, we can't start the event scheduler after
+  bootstrap either, since we want to be able to process event-related
+  SQL commands in the init file and in --bootstrap mode.
+*/
+bool in_bootstrap= FALSE;
 /**
    @brief 'grant_option' is used to indicate if privileges needs
    to be checked, in which case the lock, LOCK_grant, is used
@@ -4426,6 +4441,11 @@ we force server id to 2, but this MySQL server will not act as a slave.");
     unireg_abort(1);
   }
 
+  execute_ddl_log_recovery();
+
+  if (Events::init(opt_noacl || opt_bootstrap))
+    unireg_abort(1);
+
   if (opt_bootstrap)
   {
     select_thread_in_use= 0;                    // Allow 'kill' to work
@@ -4437,14 +4457,10 @@ we force server id to 2, but this MySQL server will not act as a slave.");
     if (read_init_file(opt_init_file))
       unireg_abort(1);
   }
-  execute_ddl_log_recovery();
 
   create_shutdown_thread();
   start_handle_manager();
 
-  if (Events::init(opt_noacl))
-    unireg_abort(1);
-
   sql_print_information(ER(ER_STARTUP),my_progname,server_version,
                         ((unix_sock == INVALID_SOCKET) ? (char*) ""
                                                        : mysqld_unix_port),
@@ -4726,6 +4742,7 @@ static void bootstrap(FILE *file)
   thd->security_ctx->master_access= ~(ulong)0;
   thd->thread_id= thd->variables.pseudo_thread_id= thread_id++;
   thread_count++;
+  in_bootstrap= TRUE;
 
   bootstrap_file=file;
 #ifndef EMBEDDED_LIBRARY			// TODO:  Enable this
@@ -4738,7 +4755,7 @@ static void bootstrap(FILE *file)
   }
   /* Wait for thread to die */
   (void) pthread_mutex_lock(&LOCK_thread_count);
-  while (thread_count)
+  while (in_bootstrap)
   {
     (void) pthread_cond_wait(&COND_thread_count,&LOCK_thread_count);
     DBUG_PRINT("quit",("One thread died (count=%u)",thread_count));

sql/sql_parse.cc

@@ -533,8 +533,9 @@ end:
 #ifndef EMBEDDED_LIBRARY
   (void) pthread_mutex_lock(&LOCK_thread_count);
   thread_count--;
-  (void) pthread_mutex_unlock(&LOCK_thread_count);
+  in_bootstrap= FALSE;
   (void) pthread_cond_broadcast(&COND_thread_count);
+  (void) pthread_mutex_unlock(&LOCK_thread_count);
   my_thread_end();
   pthread_exit(0);
 #endif