MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:March 15 2010 1:52pm
Subject:bzr commit into mysql-trunk-bugfixing branch (jon.hauglid:2994) Bug#51160
View as plain text  
#At file:///export/home/z/mysql-trunk-bugfixing-mytest/ based on revid:alik@stripped

 2994 Jon Olav Hauglid	2010-03-15
      Bug #51160 Deadlock around SET GLOBAL EVENT_SCHEDULER = ON|OFF
      
      This deadlock could occour betweeen one connection executing
      SET GLOBAL EVENT_SCHEDULER= ON and another executing SET GLOBAL
      EVENT_SCHEDULER= OFF. The bug was introduced by WL#4738.
      
      The first connection would hold LOCK_event_metadata (protecting
      the global variable) while trying to lock LOCK_global_system_variables
      starting the event scheduler thread (in THD:init()).
      
      The second connection would hold LOCK_global_system_variables
      while trying to get LOCK_event_scheduler after stopping the event
      scheduler inside event_scheduler_update().
      
      This patch fixes the problem by not using LOCK_event_metadata to
      protect the event_scheduler variable. It is still protected using
      LOCK_global_system_variables. This fixes the deadlock as it removes 
      one of the two mutexes used to produce it.
      
      However, this patch opens up the possibility that the event_scheduler
      variable and the real event_scheduler state can become out of sync
      (e.g. variable = OFF, but scheduler running). But this can only
      happen under very unlikely conditions - two concurrent SET GLOBAL
      statments, with one thread interrupted at the exact wrong moment.
      This is preferable to having the possibility of a deadlock.
      
      This patch also fixes a bug where it was possible to exit create_event()
      without releasing LOCK_event_metadata if running out of memory during
      its exection.
      
      No test case added since a repeatable test case would have required
      excessive use of new sync points. Instead we rely on the fact that
      this bug was easily reproduceable using RGQ tests.

    modified:
      sql/events.cc
      sql/events.h
      sql/sys_vars.cc
=== modified file 'sql/events.cc'
--- a/sql/events.cc	2010-02-15 11:16:49 +0000
+++ b/sql/events.cc	2010-03-15 13:52:25 +0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 2004-2006 MySQL AB, 2008-2009 Sun Microsystems, Inc
+/* Copyright (c) 2004, 2010, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -11,7 +11,7 @@
 
    You should have received a copy of the GNU General Public License
    along with this program; if not, write to the Free Software
-   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
+   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA */
 
 #include "mysql_priv.h"
 #include "events.h"
@@ -367,15 +367,14 @@ Events::create_event(THD *thd, Event_par
       {
         sql_print_error("Event Error: An error occurred while creating query string, "
                         "before writing it into binary log.");
-        /* Restore the state of binlog format */
-        DBUG_ASSERT(!thd->is_current_stmt_binlog_format_row());
-        if (save_binlog_row_based)
-          thd->set_current_stmt_binlog_format_row();
-        DBUG_RETURN(TRUE);
+        ret= TRUE;
+      }
+      else
+      {
+        /* If the definer is not set or set to CURRENT_USER, the value of CURRENT_USER
+           will be written into the binary log as the definer for the SQL thread. */
+        ret= write_bin_log(thd, TRUE, log_query.c_ptr(), log_query.length());
       }
-      /* If the definer is not set or set to CURRENT_USER, the value of CURRENT_USER 
-         will be written into the binary log as the definer for the SQL thread. */
-      ret= write_bin_log(thd, TRUE, log_query.c_ptr(), log_query.length());
     }
   }
   mysql_mutex_unlock(&LOCK_event_metadata);
@@ -1017,7 +1016,11 @@ Events::dump_internal_status()
   puts("LLA = Last Locked At  LUA = Last Unlocked At");
   puts("WOC = Waiting On Condition  DL = Data Locked");
 
-  mysql_mutex_lock(&LOCK_event_metadata);
+  /*
+    opt_event_scheduler should only be accessed while
+    holding LOCK_global_system_variables.
+  */
+  mysql_mutex_lock(&LOCK_global_system_variables);
   if (opt_event_scheduler == EVENTS_DISABLED)
     puts("The Event Scheduler is disabled");
   else
@@ -1026,7 +1029,7 @@ Events::dump_internal_status()
     event_queue->dump_internal_status();
   }
 
-  mysql_mutex_unlock(&LOCK_event_metadata);
+  mysql_mutex_unlock(&LOCK_global_system_variables);
   DBUG_VOID_RETURN;
 }
 

=== modified file 'sql/events.h'
--- a/sql/events.h	2010-01-07 05:42:07 +0000
+++ b/sql/events.h	2010-03-15 13:52:25 +0000
@@ -1,6 +1,6 @@
 #ifndef _EVENT_H_
 #define _EVENT_H_
-/* Copyright (C) 2004-2006 MySQL AB, 2008-2009 Sun Microsystems, Inc
+/* Copyright (c) 2004, 2010, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -13,7 +13,7 @@
 
    You should have received a copy of the GNU General Public License
    along with this program; if not, write to the Free Software
-   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
+   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA */
 
 /**
   @defgroup Event_Scheduler Event Scheduler
@@ -83,6 +83,7 @@ public:
     See sys_var.cc
   */
   enum enum_opt_event_scheduler { EVENTS_OFF, EVENTS_ON, EVENTS_DISABLED };
+  /* Protected using LOCK_global_system_variables only. */
   static uint opt_event_scheduler;
   static mysql_mutex_t LOCK_event_metadata;
   static bool check_if_system_tables_error();
@@ -107,9 +108,6 @@ public:
   destroy_mutexes();
 
   static bool
-  switch_event_scheduler_state(enum enum_opt_event_scheduler new_state);
-
-  static bool
   create_event(THD *thd, Event_parse_data *parse_data, bool if_exists);
 
   static bool

=== modified file 'sql/sys_vars.cc'
--- a/sql/sys_vars.cc	2010-03-05 13:08:21 +0000
+++ b/sql/sys_vars.cc	2010-03-15 13:52:25 +0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 2002-2006 MySQL AB, 2009-2010 Sun Microsystems, Inc.
+/* Copyright (c) 2002, 2010, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -11,7 +11,7 @@
 
    You should have received a copy of the GNU General Public License
    along with this program; if not, write to the Free Software
-   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
+   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA */
 
 /*
   How to add new variables:
@@ -647,32 +647,40 @@ static bool event_scheduler_check(sys_va
 }
 static bool event_scheduler_update(sys_var *self, THD *thd, enum_var_type type)
 {
+  uint opt_event_scheduler_value= Events::opt_event_scheduler;
   mysql_mutex_unlock(&LOCK_global_system_variables);
   /*
     Events::start() is heavyweight. In particular it creates a new THD,
     which takes LOCK_global_system_variables internally.
     Thus we have to release it here.
     We need to re-take it before returning, though.
-    And we need to take it *without* holding Events::LOCK_event_metadata.
+
+    Note that since we release LOCK_global_system_variables before calling
+    start/stop, there is a possibility that the server variable
+    can become out of sync with the real event scheduler state.
+
+    This can happen with two concurrent statments if the first gets
+    interrupted after start/stop but before retaking
+    LOCK_global_system_variables. However, this problem should be quite
+    rare and it's difficult to avoid it without opening up possibilities
+    for deadlocks. See bug#51160.
   */
-  bool ret= Events::opt_event_scheduler == Events::EVENTS_ON
+  bool ret= opt_event_scheduler_value == Events::EVENTS_ON
             ? Events::start()
             : Events::stop();
-  mysql_mutex_unlock(&Events::LOCK_event_metadata);
   mysql_mutex_lock(&LOCK_global_system_variables);
-  mysql_mutex_lock(&Events::LOCK_event_metadata);
   if (ret)
     my_error(ER_EVENT_SET_VAR_ERROR, MYF(0));
   return ret;
 }
-static PolyLock_mutex PLock_event_metadata(&Events::LOCK_event_metadata);
+
 static Sys_var_enum Sys_event_scheduler(
        "event_scheduler", "Enable the event scheduler. Possible values are "
        "ON, OFF, and DISABLED (keep the event scheduler completely "
        "deactivated, it cannot be activated run-time)",
        GLOBAL_VAR(Events::opt_event_scheduler), CMD_LINE(OPT_ARG),
        event_scheduler_names, DEFAULT(Events::EVENTS_OFF),
-       &PLock_event_metadata, NOT_IN_BINLOG,
+       NO_MUTEX_GUARD, NOT_IN_BINLOG,
        ON_CHECK(event_scheduler_check), ON_UPDATE(event_scheduler_update));
 #endif
 


Attachment: [text/bzr-bundle] bzr/jon.hauglid@sun.com-20100315135225-ekamzhlhvpdpsa3u.bundle
Thread
bzr commit into mysql-trunk-bugfixing branch (jon.hauglid:2994) Bug#51160Jon Olav Hauglid15 Mar