Fix for BUG#12228: SP cache code:

* Cleanup SP Cache code, now SP Cache only deletes sp_head objects in 
  sp_cache_flush_obsolete() invalidates all pointers to routines in the cache.
* Use new SP Cache use contract in the code.

There is no test case because it doesn't seem to be possible to cause thread races to end
the same way they end in heavy-load test. This patch removes the crash in heavy test.


mysql-test/r/type_bit.result:
  Drop the tables this test tries to create
mysql-test/r/view.result:
  Drop function this test creates
mysql-test/t/type_bit.test:
  Drop the tables this test tries to create
mysql-test/t/view.test:
  Drop function this test creates
sql/sp.cc:
  Fix for BUG#12228: When a routine is deleted/modified, invalidate all cached SPs in all
  threads. We need to do so because sp_lex_keeper::{prelocking_tables, query_tables_own_last}
  in one SP may depend on another SP sp_lex_keeper::m_lex is using.
sql/sp_cache.cc:
  Fix for BUG#12228:
  * Move class sp_cache to here from sp_cache.h, document the functions.
  * sp_cache_insert, sp_cache_remove, sp_cache_invalidate and sp_cache_lookup must not delete
    sp_head* objects as they may be called during SP execution when sp_head objects are used.
  * Added sp_cache_flush_obsolete() function that may delete sp_head objects.
  * Removed sp_cache_remove as there is no need for it now - when we change one SP we should
    invalidate all other SPs, because sp_lex_keeper::{prelocking_tables, 
    query_tables_own_last} from one SP depend on content of another SP (used in 
    sp_lex_keeper::m_lex).
sql/sp_cache.h:
  Fix for BUG#12228:
  * Move class sp_cache to sp_cache.cc it is not needed in .h file
  * Added comments
sql/sql_parse.cc:
  Fix for BUG#12228: Call new sp_cache_flush_obsolete() function before running the query
sql/sql_prepare.cc:
  Fix for BUG#12228: Call new sp_cache_flush_obsolete() function before preparing/executing a PS

1 files changed, 143 insertions, 50 deletions

diff --git a/sql/sp_cache.cc b/sql/sp_cache.cc
index c8f0ed6ba2d..135000d1443 100644
--- a/sql/sp_cache.cc
+++ b/sql/sp_cache.cc
@@ -24,17 +24,78 @@
 static pthread_mutex_t Cversion_lock;
 static ulong Cversion = 0;
 
-void
-sp_cache_init()
+
+/*
+  Cache of stored routines. 
+*/
+
+class sp_cache
+{
+public:
+  ulong version;
+
+  sp_cache();
+  ~sp_cache();
+
+  inline void insert(sp_head *sp)
+  {
+    /* TODO: why don't we check return value? */
+    my_hash_insert(&m_hashtable, (const byte *)sp);
+  }
+
+  inline sp_head *lookup(char *name, uint namelen)
+  {
+    return (sp_head *)hash_search(&m_hashtable, (const byte *)name, namelen);
+  }
+
+#ifdef NOT_USED
+  inline bool remove(char *name, uint namelen)
+  {
+    sp_head *sp= lookup(name, namelen);
+    if (sp)
+    {
+      hash_delete(&m_hashtable, (byte *)sp);
+      return TRUE;
+    }
+    return FALSE;
+  }
+#endif 
+
+  inline void remove_all()
+  {
+    cleanup();
+    init();
+  }
+
+private:
+  void init();
+  void cleanup();
+
+  /* All routines in this cache */
+  HASH m_hashtable;
+}; // class sp_cache
+
+
+/* Initialize the SP caching once at startup */
+
+void sp_cache_init()
 {
   pthread_mutex_init(&Cversion_lock, MY_MUTEX_INIT_FAST);
 }
 
-void
-sp_cache_clear(sp_cache **cp)
+
+/*
+  Clear the cache *cp and set *cp to NULL.
+  SYNOPSIS
+    sp_cache_clear()
+      cp  Pointer to cache to clear
+  NOTE
+    This function doesn't invalidate other caches.
+*/
+
+void sp_cache_clear(sp_cache **cp)
 {
   sp_cache *c= *cp;
-
   if (c)
   {
     delete c;
@@ -42,85 +103,118 @@ sp_cache_clear(sp_cache **cp)
   }
 }
 
-void
-sp_cache_insert(sp_cache **cp, sp_head *sp)
+
+/*
+  Insert a routine into the cache.
+
+  SYNOPSIS
+    sp_cache_insert()
+      cp  The cache to put routine into
+      sp  Routine to insert.
+*/
+
+void sp_cache_insert(sp_cache **cp, sp_head *sp)
 {
   sp_cache *c= *cp;
 
   if (! c)
-    c= new sp_cache();
-  if (c)
   {
     ulong v;
-
+    c= new sp_cache();
     pthread_mutex_lock(&Cversion_lock); // LOCK
     v= Cversion;
     pthread_mutex_unlock(&Cversion_lock); // UNLOCK
-
-    if (c->version < v)
-    {
-      if (*cp)
-	c->remove_all();
-      c->version= v;
-    }
+    c->version= v;
+  }
+  if (c)
+  {
+    DBUG_PRINT("info",("sp_cache: inserting: %*s", sp->m_qname.length, sp->m_qname.str));
     c->insert(sp);
     if (*cp == NULL)
       *cp= c;
   }
 }
 
-sp_head *
-sp_cache_lookup(sp_cache **cp, sp_name *name)
+
+/* 
+  Look up a routine in the cache.
+  SYNOPSIS
+    sp_cache_lookup()
+      cp    Cache to look into
+      name  Name of rutine to find
+      
+  NOTE
+    An obsolete (but not more obsolete then since last
+    sp_cache_flush_obsolete call) routine may be returned.
+
+  RETURN 
+    The routine or
+    NULL if the routine not found.
+*/
+
+sp_head *sp_cache_lookup(sp_cache **cp, sp_name *name)
 {
-  ulong v;
   sp_cache *c= *cp;
-
-  if (! c)
+  if (!c)
     return NULL;
+  return c->lookup(name->m_qname.str, name->m_qname.length);
+}
+
+
+/*
+  Invalidate all routines in all caches.
+
+  SYNOPSIS
+    sp_cache_invalidate()
+      
+  NOTE
+    This is called when a VIEW definition is modifed. We can't destroy sp_head
+    objects here as one may modify VIEW definitions from prelocking-free SPs.
+*/
 
+void sp_cache_invalidate()
+{
+  DBUG_PRINT("info",("sp_cache: invalidating"));
   pthread_mutex_lock(&Cversion_lock); // LOCK
-  v= Cversion;
+  Cversion++;
   pthread_mutex_unlock(&Cversion_lock); // UNLOCK
-
-  if (c->version < v)
-  {
-    c->remove_all();
-    c->version= v;
-    return NULL;
-  }
-  return c->lookup(name->m_qname.str, name->m_qname.length);
 }
 
-bool
-sp_cache_remove(sp_cache **cp, sp_name *name)
+
+/*
+  Remove out-of-date SPs from the cache. 
+  
+  SYNOPSIS
+    sp_cache_flush_obsolete()
+      cp  Cache to flush
+
+  NOTE
+    This invalidates pointers to sp_head objects this thread uses.
+    In practice that means 'dont call this function when inside SP'.
+*/
+
+void sp_cache_flush_obsolete(sp_cache **cp)
 {
   sp_cache *c= *cp;
-  bool found= FALSE;
-
   if (c)
   {
     ulong v;
-
     pthread_mutex_lock(&Cversion_lock); // LOCK
-    v= Cversion++;
+    v= Cversion;
     pthread_mutex_unlock(&Cversion_lock); // UNLOCK
-
     if (c->version < v)
+    {
+      DBUG_PRINT("info",("sp_cache: deleting all functions"));
+      /* We need to delete all elements. */
       c->remove_all();
-    else
-      found= c->remove(name->m_qname.str, name->m_qname.length);
-    c->version= v+1;
+      c->version= v;
+    }
   }
-  return found;
 }
 
-void
-sp_cache_invalidate()
-{
-  pthread_mutex_lock(&Cversion_lock); // LOCK
-  Cversion++;
-  pthread_mutex_unlock(&Cversion_lock); // UNLOCK
-}
+/*************************************************************************
+  Internal functions 
+ *************************************************************************/
 
 static byte *
 hash_get_key_for_sp_head(const byte *ptr, uint *plen,
@@ -136,7 +230,6 @@ static void
 hash_free_sp_head(void *p)
 {
   sp_head *sp= (sp_head *)p;
-
   delete sp;
 }