Backport of revno: 3685

Bug #48210 FLUSH TABLES WITH READ LOCK deadlocks
           against concurrent CREATE PROCEDURE

This deadlock occured between
a) CREATE PROCEDURE (or other commands listed below)
b) FLUSH TABLES WITH READ LOCK

If the execution of them happened in the following order:
- a) opens a table (e.g. mysql.proc)
- b) locks the global read lock (or GRL)
- a) sleeps inside wait_if_global_read_lock()
- b) increases refresh_version and sleeps waiting 
     for old tables to go away

Note that a) must start waiting on the GRL before FLUSH increases
refresh_version. Otherwise a) won't wait on the GRL and instead
close its tables for reopen, allowing FLUSH to complete and thus
avoid the deadlock.

With this patch the deadlock is avoided by making CREATE PROCEDURE
acquire a protection against global read locks before it starts
executing. This means that FLUSH TABLES WITH READ LOCK will have
to wait until CREATE PROCEDURE completes before acquiring the global
read lock, thereby avoiding the deadlock.

This is implemented by introducing a new SQL command flag called
CF_PROTECT_AGAINST_GRL. Commands marked with this flag will
acquire a GRL protection in the beginning of mysql_execute_command().
This patch adds the flag to CREATE, ALTER and DROP for PROCEDURE
and FUNCTION, as well as CREATE USER, DROP USER, RENAME USER and 
REVOKE ALL. All these commands either call open_grant_tables() or
open_system_table_for_updated() which make them susceptible for
this deadlock.

The patch also adds the CF_PROTECT_AGAINST_GRL flag to a number
of commands that previously acquired GRL protection in their
respective SQLCOM case in mysql_execute_command().

Test case that checks for GRL protection for CREATE PROCEDURE
and CREATE USER added to mdl_sync.test.

1 files changed, 26 insertions, 1 deletions

diff --git a/sql/lock.cc b/sql/lock.cc
index b64f7b0b44c..98217254240 100644
--- a/sql/lock.cc
+++ b/sql/lock.cc
@@ -74,6 +74,7 @@
 */
 
 #include "mysql_priv.h"
+#include "debug_sync.h"
 #include <hash.h>
 #include <assert.h>
 
@@ -1119,9 +1120,33 @@ bool lock_global_read_lock(THD *thd)
   if (!thd->global_read_lock)
   {
     const char *old_message;
+    const char *new_message= "Waiting to get readlock";
     (void) pthread_mutex_lock(&LOCK_global_read_lock);
+
+#if defined(ENABLED_DEBUG_SYNC)
+    /*
+      The below sync point fires if we have to wait for
+      protect_against_global_read_lock.
+
+      WARNING: Beware to use WAIT_FOR with this sync point. We hold
+      LOCK_global_read_lock here.
+
+      Call the sync point before calling enter_cond() as it does use
+      enter_cond() and exit_cond() itself if a WAIT_FOR action is
+      executed in spite of the above warning.
+
+      Pre-set proc_info so that it is available immediately after the
+      sync point sends a SIGNAL. This makes tests more reliable.
+    */
+    if (protect_against_global_read_lock)
+    {
+      thd_proc_info(thd, new_message);
+      DEBUG_SYNC(thd, "wait_lock_global_read_lock");
+    }
+#endif /* defined(ENABLED_DEBUG_SYNC) */
+
     old_message=thd->enter_cond(&COND_global_read_lock, &LOCK_global_read_lock,
-                                "Waiting to get readlock");
+                                new_message);
     DBUG_PRINT("info",
 	       ("waiting_for: %d  protect_against: %d",
 		waiting_for_read_lock, protect_against_global_read_lock));