Merge mysql.com:/home/psergey/mysql-5.0-bug12228-r4

into mysql.com:/home/psergey/mysql-5.0-bug12228-r5

mysql-test/r/sp-threads.result

@@ -37,6 +37,7 @@ Id	User	Host	db	Command	Time	State	Info
 #	root	localhost	test	Sleep	#		NULL
 #	root	localhost	test	Query	#	Locked	update t1, t2 set val= 1 where id1=id2
 #	root	localhost	test	Query	#	NULL	show processlist
+#	root	localhost	test	Sleep	#		NULL
 unlock tables;
 drop procedure bug9486;
 drop table t1, t2;
@@ -64,3 +65,27 @@ insert into t1 (select f from v1);
 drop function bug11554;
 drop table t1;
 drop view v1;
+drop procedure if exists p1;
+drop procedure if exists p2;
+create table t1 (s1 int)|
+create procedure p1() select * from t1|
+create procedure p2()
+begin
+insert into t1 values (1);
+call p1();
+select * from t1;
+end|
+use test;
+lock table t1 write;
+ call p2();
+use test;
+drop procedure p1;
+create procedure p1() select * from t1;
+unlock tables;
+s1
+1
+s1
+1
+drop procedure p1;
+drop procedure p2;
+drop table t1;

mysql-test/r/type_bit.result

@@ -34,7 +34,7 @@ select 0 + b'1111111111111111';
 select 0 + b'1000000000000001';
 0 + b'1000000000000001'
 32769
-drop table if exists t1;
+drop table if exists t1,t2;
 create table t1 (a bit(65));
 ERROR 42000: Display width out of range for column 'a' (max = 64)
 create table t1 (a bit(0));

mysql-test/r/view.result

@@ -1880,6 +1880,8 @@ test.v5	check	error	View 'test.v5' references invalid table(s) or column(s) or f
 test.v6	check	status	OK
 drop view v1, v2, v3, v4, v5, v6;
 drop table t2;
+drop function if exists f1;
+drop function if exists f2;
 CREATE TABLE t1 (col1 time);
 CREATE TABLE t2 (col1 time);
 CREATE TABLE t3 (col1 time);

mysql-test/t/sp-threads.test

@@ -5,6 +5,7 @@
 
 connect (con1root,localhost,root,,);
 connect (con2root,localhost,root,,);
+connect (con3root,localhost,root,,);
 
 connection con1root;
 use test;
@@ -130,6 +131,48 @@ drop function bug11554;
 drop table t1;
 drop view v1;
 
+
+# BUG#12228 
+--disable_warnings
+drop procedure if exists p1;
+drop procedure if exists p2;
+--enable_warnings
+
+connection con1root;
+delimiter |;
+create table t1 (s1 int)|
+create procedure p1() select * from t1|
+create procedure p2()
+begin
+  insert into t1 values (1);
+  call p1();
+  select * from t1;
+end|
+delimiter ;|
+
+connection con2root;
+use test;
+lock table t1 write;
+
+connection con1root;
+send call p2();
+
+connection con3root;
+use test;
+drop procedure p1;
+create procedure p1() select * from t1;
+
+connection con2root;
+unlock tables;
+
+connection con1root;
+# Crash will be here if we hit BUG#12228 
+reap;
+
+drop procedure p1;
+drop procedure p2;
+drop table t1;
+
 #
 # BUG#NNNN: New bug synopsis
 #

mysql-test/t/type_bit.test

@@ -16,7 +16,7 @@ select 0 + b'1111111111111111';
 select 0 + b'1000000000000001';
 
 --disable_warnings
-drop table if exists t1;
+drop table if exists t1,t2;
 --enable_warnings
 
 --error 1439

mysql-test/t/view.test

@@ -1711,6 +1711,10 @@ CHECK TABLE v1, v2, v3, v4, v5, v6;
 drop view v1, v2, v3, v4, v5, v6;
 drop table t2;
 
+--disable_warnings
+drop function if exists f1;
+drop function if exists f2;
+--enable_warnings
 CREATE TABLE t1 (col1 time);
 CREATE TABLE t2 (col1 time);
 CREATE TABLE t3 (col1 time);

sql/sp.cc

@@ -989,13 +989,11 @@ int
 sp_drop_procedure(THD *thd, sp_name *name)
 {
   int ret;
-  bool found;
   DBUG_ENTER("sp_drop_procedure");
   DBUG_PRINT("enter", ("name: %*s", name->m_name.length, name->m_name.str));
 
-  found= sp_cache_remove(&thd->sp_proc_cache, name);
   ret= db_drop_routine(thd, TYPE_ENUM_PROCEDURE, name);
-  if (!found && !ret)
+  if (!ret)
     sp_cache_invalidate();
   DBUG_RETURN(ret);
 }
@@ -1005,13 +1003,11 @@ int
 sp_update_procedure(THD *thd, sp_name *name, st_sp_chistics *chistics)
 {
   int ret;
-  bool found;
   DBUG_ENTER("sp_update_procedure");
   DBUG_PRINT("enter", ("name: %*s", name->m_name.length, name->m_name.str));
 
-  found= sp_cache_remove(&thd->sp_proc_cache, name);
   ret= db_update_routine(thd, TYPE_ENUM_PROCEDURE, name, chistics);
-  if (!found && !ret)
+  if (!ret)
     sp_cache_invalidate();
   DBUG_RETURN(ret);
 }
@@ -1102,13 +1098,11 @@ int
 sp_drop_function(THD *thd, sp_name *name)
 {
   int ret;
-  bool found;
   DBUG_ENTER("sp_drop_function");
   DBUG_PRINT("enter", ("name: %*s", name->m_name.length, name->m_name.str));
 
-  found= sp_cache_remove(&thd->sp_func_cache, name);
   ret= db_drop_routine(thd, TYPE_ENUM_FUNCTION, name);
-  if (!found && !ret)
+  if (!ret)
     sp_cache_invalidate();
   DBUG_RETURN(ret);
 }
@@ -1118,13 +1112,11 @@ int
 sp_update_function(THD *thd, sp_name *name, st_sp_chistics *chistics)
 {
   int ret;
-  bool found;
   DBUG_ENTER("sp_update_procedure");
   DBUG_PRINT("enter", ("name: %*s", name->m_name.length, name->m_name.str));
 
-  found= sp_cache_remove(&thd->sp_func_cache, name);
   ret= db_update_routine(thd, TYPE_ENUM_FUNCTION, name, chistics);
-  if (!found && !ret)
+  if (!ret)
     sp_cache_invalidate();
   DBUG_RETURN(ret);
 }

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,121 @@ 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.
+      
+  TODO: Perhaps it will be more straightforward if in case we returned an 
+        error from this function when we couldn't allocate sp_cache. (right
+        now failure to put routine into cache will cause a 'SP not found'
+        error to be reported at some later time)
+*/
+
+void sp_cache_insert(sp_cache **cp, sp_head *sp)
 {
   sp_cache *c= *cp;
 
-  if (! c)
-    c= new sp_cache();
-  if (c)
+  if (!c && (c= new sp_cache()))
   {
-    ulong v;
-
     pthread_mutex_lock(&Cversion_lock); // LOCK
-    v= Cversion;
+    c->version= Cversion;
     pthread_mutex_unlock(&Cversion_lock); // UNLOCK
-
-    if (c->version < v)
-    {
-      if (*cp)
-	c->remove_all();
-      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 +233,6 @@ static void
 hash_free_sp_head(void *p)
 {
   sp_head *sp= (sp_head *)p;
-
   delete sp;
 }
 

sql/sp_cache.h

@@ -25,94 +25,39 @@
 /*
   Stored procedures/functions cache. This is used as follows:
    * Each thread has its own cache.
-   * Each sp_head object is put into its thread cache before it is used, and 
+   * Each sp_head object is put into its thread cache before it is used, and
      then remains in the cache until deleted.
 */
 
 class sp_head;
 class sp_cache;
 
-/* Initialize the SP caching once at startup */
-void sp_cache_init();
+/*
+  Cache usage scenarios:
+  1. Application-wide init:
+    sp_cache_init();
+
+  2. SP execution in thread:
+  2.1 While holding sp_head* pointers:
+  
+    // look up a routine in the cache (no checks if it is up to date or not)
+    sp_cache_lookup(); 
+    
+    sp_cache_insert();
+    sp_cache_invalidate();
+  
+  2.2 When not holding any sp_head* pointers:
+    sp_cache_flush_obsolete();
+  
+  3. Before thread exit:
+    sp_cache_clear();
+*/
 
-/* Clear the cache *cp and set *cp to NULL */
+void sp_cache_init();
 void sp_cache_clear(sp_cache **cp);
-
-/* Insert an SP into cache. If 'cp' points to NULL, it's set to a new cache */
 void sp_cache_insert(sp_cache **cp, sp_head *sp);
-
-/* Lookup an SP in cache */
 sp_head *sp_cache_lookup(sp_cache **cp, sp_name *name);
-
-/* 
-  Remove an SP from cache, and also bump the Cversion number so all other 
-  caches are invalidated. 
-  Returns true if something was removed.
-*/
-bool sp_cache_remove(sp_cache **cp, sp_name *name);
-
-/* Invalidate all existing SP caches by bumping Cversion number. */
 void sp_cache_invalidate();
-
-
-/*
- *
- * The cache class. Don't use this directly, use the C API above
- *
- */
-
-class sp_cache
-{
-public:
-
-  ulong version;
-
-  sp_cache();
-
-  ~sp_cache();
-
-  void
-  init();
-
-  void
-  cleanup();
-
-  inline void
-  insert(sp_head *sp)
-  {
-    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);
-  }
-
-  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;
-  }
-
-  inline void
-  remove_all()
-  {
-    cleanup();
-    init();
-  }
-
-private:
-
-  HASH m_hashtable;
-
-}; // class sp_cache
+void sp_cache_flush_obsolete(sp_cache **cp);
 
 #endif /* _SP_CACHE_H_ */

sql/sql_parse.cc

@@ -5340,11 +5340,12 @@ void mysql_init_multi_delete(LEX *lex)
 void mysql_parse(THD *thd, char *inBuf, uint length)
 {
   DBUG_ENTER("mysql_parse");
-
   mysql_init_query(thd, (uchar*) inBuf, length);
   if (query_cache_send_result_to_client(thd, inBuf, length) <= 0)
   {
     LEX *lex= thd->lex;
+    sp_cache_flush_obsolete(&thd->sp_proc_cache);
+    sp_cache_flush_obsolete(&thd->sp_func_cache);
     if (!yyparse((void *)thd) && ! thd->is_fatal_error)
     {
 #ifndef NO_EMBEDDED_ACCESS_CHECKS

sql/sql_prepare.cc

@@ -73,6 +73,7 @@ Long data handling:
 #include <m_ctype.h>  // for isspace()
 #include "sp_head.h"
 #include "sp.h"
+#include "sp_cache.h"
 #ifdef EMBEDDED_LIBRARY
 /* include MYSQL_BIND headers */
 #include <mysql.h>
@@ -1783,6 +1784,9 @@ bool mysql_stmt_prepare(THD *thd, char *packet, uint packet_length,
   lex= thd->lex;
   lex->safe_to_cache_query= 0;
 
+  sp_cache_flush_obsolete(&thd->sp_proc_cache);
+  sp_cache_flush_obsolete(&thd->sp_func_cache);
+
   error= yyparse((void *)thd) || thd->is_fatal_error ||
          thd->net.report_error || init_param_array(stmt);
   /*
@@ -2060,6 +2064,8 @@ void mysql_stmt_execute(THD *thd, char *packet, uint packet_length)
   thd->protocol= stmt->protocol;                // Switch to binary protocol
   if (!(specialflag & SPECIAL_NO_PRIOR))
     my_pthread_setprio(pthread_self(),QUERY_PRIOR);
+  sp_cache_flush_obsolete(&thd->sp_proc_cache);
+  sp_cache_flush_obsolete(&thd->sp_func_cache);
   mysql_execute_command(thd);
   if (!(specialflag & SPECIAL_NO_PRIOR))
     my_pthread_setprio(pthread_self(), WAIT_PRIOR);