Improvements to the SQLITE_MISUSE detection logic. Also added test cases

for this logic, including the new test file "misuse.test". (CVS 559)

FossilOrigin-Name: f42907ce457e012592f8c043dc6c915e87258b35

6 files changed, 235 insertions, 78 deletions

diff --git a/src/main.c b/src/main.c
index c66f0e8c8..56865f13c 100644
--- a/src/main.c
+++ b/src/main.c
@@ -14,7 +14,7 @@
 ** other files are for internal use by SQLite and should not be
 ** accessed by users of the library.
 **
-** $Id: main.c,v 1.70 2002/05/10 05:44:56 drh Exp $
+** $Id: main.c,v 1.71 2002/05/10 13:14:07 drh Exp $
 */
 #include "sqliteInt.h"
 #include "os.h"
@@ -435,7 +435,7 @@ int sqlite_changes(sqlite *db){
 */
 void sqlite_close(sqlite *db){
   HashElem *i;
-  if( sqliteSafetyOn(db) ){ return; }
+  if( sqliteSafetyCheck(db) || sqliteSafetyOn(db) ){ return; }
   db->magic = SQLITE_MAGIC_CLOSED;
   sqliteBtreeClose(db->pBe);
   clearHashTable(db, 0);
@@ -530,7 +530,7 @@ int sqlite_exec(
   Parse sParse;
 
   if( pzErrMsg ) *pzErrMsg = 0;
-  if( sqliteSafetyOn(db) ){ return SQLITE_MISUSE; }
+  if( sqliteSafetyOn(db) ) goto exec_misuse;
   if( (db->flags & SQLITE_Initialized)==0 ){
     int rc = sqliteInit(db, pzErrMsg);
     if( rc!=SQLITE_OK ){
@@ -560,47 +560,50 @@ int sqlite_exec(
     clearHashTable(db, 1);
   }
   db->recursionDepth--;
-  if( sqliteSafetyOff(db) ){ sParse.rc = SQLITE_MISUSE; }
+  if( sqliteSafetyOff(db) ) goto exec_misuse;
   return sParse.rc;
-}
 
-/*
-** Change the magic from SQLITE_MAGIC_OPEN to SQLITE_MAGIC_BUSY.
-** Return an error (non-zero) if the magic was not SQLITE_MAGIC_OPEN
-** when this routine is called.
-**
-** This routine is a attempt to detect if two threads attempt
-** to use the same sqlite* pointer at the same time.  There is a
-** race condition so it is possible that the error is not detected.
-** But usually the problem will be seen.  The result will be an
-** error which can be used to debugging the application that is
-** using SQLite incorrectly.
-*/
-int sqliteSafetyOn(sqlite *db){
-  if( db->magic==SQLITE_MAGIC_OPEN ){
-    db->magic = SQLITE_MAGIC_BUSY;
-    return 0;
-  }else{
-    db->magic = SQLITE_MAGIC_ERROR;
-    db->flags |= SQLITE_Interrupt;
-    return 1;
+exec_misuse:
+  if( pzErrMsg ){
+    *pzErrMsg = 0;
+    sqliteSetString(pzErrMsg, sqlite_error_string(SQLITE_MISUSE), 0);
+    sqliteStrRealloc(pzErrMsg);
   }
+  return SQLITE_MISUSE;
 }
 
 /*
-** Change the magic from SQLITE_MAGIC_BUSY to SQLITE_MAGIC_OPEN.
-** Return an error (non-zero) if the magic was not SQLITE_MAGIC_BUSY
-** when this routine is called.
+** Return a static string that describes the kind of error specified in the
+** argument.
 */
-int sqliteSafetyOff(sqlite *db){
-  if( db->magic==SQLITE_MAGIC_BUSY ){
-    db->magic = SQLITE_MAGIC_OPEN;
-    return 0;
-  }else{
-    db->magic = SQLITE_MAGIC_ERROR;
-    db->flags |= SQLITE_Interrupt;
-    return 1;
+const char *sqlite_error_string(int rc){
+  const char *z;
+  switch( rc ){
+    case SQLITE_OK:         z = "not an error";                          break;
+    case SQLITE_ERROR:      z = "SQL logic error or missing database";   break;
+    case SQLITE_INTERNAL:   z = "internal SQLite implementation flaw";   break;
+    case SQLITE_PERM:       z = "access permission denied";              break;
+    case SQLITE_ABORT:      z = "callback requested query abort";        break;
+    case SQLITE_BUSY:       z = "database is locked";                    break;
+    case SQLITE_LOCKED:     z = "database table is locked";              break;
+    case SQLITE_NOMEM:      z = "out of memory";                         break;
+    case SQLITE_READONLY:   z = "attempt to write a readonly database";  break;
+    case SQLITE_INTERRUPT:  z = "interrupted";                           break;
+    case SQLITE_IOERR:      z = "disk I/O error";                        break;
+    case SQLITE_CORRUPT:    z = "database disk image is malformed";      break;
+    case SQLITE_NOTFOUND:   z = "table or record not found";             break;
+    case SQLITE_FULL:       z = "database is full";                      break;
+    case SQLITE_CANTOPEN:   z = "unable to open database file";          break;
+    case SQLITE_PROTOCOL:   z = "database locking protocol failure";     break;
+    case SQLITE_EMPTY:      z = "table contains no data";                break;
+    case SQLITE_SCHEMA:     z = "database schema has changed";           break;
+    case SQLITE_TOOBIG:     z = "too much data for one table row";       break;
+    case SQLITE_CONSTRAINT: z = "constraint failed";                     break;
+    case SQLITE_MISMATCH:   z = "datatype mismatch";                     break;
+    case SQLITE_MISUSE:     z = "library routine called out of sequence";break;
+    default:                z = "unknown error";                         break;
   }
+  return z;
 }
 
 /*
@@ -718,7 +721,7 @@ int sqlite_create_function(
   void *pUserData      /* User data */
 ){
   FuncDef *p;
-  if( db==0 || zName==0 ) return 1;
+  if( db==0 || zName==0 || sqliteSafetyCheck(db) ) return 1;
   p = sqliteFindFunction(db, zName, strlen(zName), nArg, 1);
   if( p==0 ) return 1;
   p->xFunc = xFunc;
@@ -736,7 +739,7 @@ int sqlite_create_aggregate(
   void *pUserData      /* User data */
 ){
   FuncDef *p;
-  if( db==0 || zName==0 ) return 1;
+  if( db==0 || zName==0 || sqliteSafetyCheck(db) ) return 1;
   p = sqliteFindFunction(db, zName, strlen(zName), nArg, 1);
   if( p==0 ) return 1;
   p->xFunc = 0;
diff --git a/src/sqliteInt.h b/src/sqliteInt.h
index fbdb47dc9..f8251bbd1 100644
--- a/src/sqliteInt.h
+++ b/src/sqliteInt.h
@@ -11,7 +11,7 @@
 *************************************************************************
 ** Internal interface definitions for SQLite.
 **
-** @(#) $Id: sqliteInt.h,v 1.106 2002/05/10 05:44:56 drh Exp $
+** @(#) $Id: sqliteInt.h,v 1.107 2002/05/10 13:14:07 drh Exp $
 */
 #include "sqlite.h"
 #include "hash.h"
@@ -661,3 +661,4 @@ FuncDef *sqliteFindFunction(sqlite*,const char*,int,int,int);
 void sqliteRegisterBuildinFunctions(sqlite*);
 int sqliteSafetyOn(sqlite*);
 int sqliteSafetyOff(sqlite*);
+int sqliteSafetyCheck(sqlite*);
diff --git a/src/tclsqlite.c b/src/tclsqlite.c
index 045c18d78..7df3f7741 100644
--- a/src/tclsqlite.c
+++ b/src/tclsqlite.c
@@ -11,7 +11,7 @@
 *************************************************************************
 ** A TCL Interface to SQLite
 **
-** $Id: tclsqlite.c,v 1.31 2002/04/12 10:08:59 drh Exp $
+** $Id: tclsqlite.c,v 1.32 2002/05/10 13:14:07 drh Exp $
 */
 #ifndef NO_TCL     /* Omit this whole file if TCL is unavailable */
 
@@ -552,10 +552,18 @@ static int DbMain(void *cd, Tcl_Interp *interp, int argc, char **argv){
     return TCL_ERROR;
   }
   Tcl_CreateObjCommand(interp, argv[1], DbObjCmd, (char*)p, DbDeleteCmd);
+
+  /* If compiled with SQLITE_TEST turned on, then register the "md5sum"
+  ** SQL function and return an integer which is the memory address of
+  ** the underlying sqlite* pointer.
+  */
 #ifdef SQLITE_TEST
   {
-     extern void Md5_Register(sqlite*);
-     Md5_Register(p->db);
+    char zBuf[40];
+    extern void Md5_Register(sqlite*);
+    Md5_Register(p->db);
+    sprintf(zBuf, "%d", (int)p->db);
+    Tcl_AppendResult(interp, zBuf, 0);
   }
 #endif  
   return TCL_OK;
diff --git a/src/test1.c b/src/test1.c
index 217d1f9d9..852824b52 100644
--- a/src/test1.c
+++ b/src/test1.c
@@ -13,7 +13,7 @@
 ** is not included in the SQLite library.  It is used for automated
 ** testing of the SQLite library.
 **
-** $Id: test1.c,v 1.7 2002/03/11 02:06:13 drh Exp $
+** $Id: test1.c,v 1.8 2002/05/10 13:14:07 drh Exp $
 */
 #include "sqliteInt.h"
 #include "tcl.h"
@@ -198,6 +198,117 @@ static int sqlite_test_close(
 }
 
 /*
+** Implementation of the x_coalesce() function.
+** Return the first argument non-NULL argument.
+*/
+static void ifnullFunc(sqlite_func *context, int argc, const char **argv){
+  int i;
+  for(i=0; i<argc; i++){
+    if( argv[i] ){
+      sqlite_set_result_string(context, argv[i], -1);
+      break;
+    }
+  }
+}
+
+/*
+** Implementation of the x_sqlite_exec() function.  This function takes
+** a single argument and attempts to execute that argument as SQL code.
+** This is illegal and shut set the SQLITE_MISUSE flag on the database.
+** 
+** This routine simulates the effect of having two threads attempt to
+** use the same database at the same time.
+*/
+static void sqliteExecFunc(sqlite_func *context, int argc, const char **argv){
+  sqlite_exec((sqlite*)sqlite_user_data(context), argv[0], 0, 0, 0);
+}
+
+/*
+** Usage:  sqlite_test_create_function DB
+**
+** Call the sqlite_create_function API on the given database in order
+** to create a function named "x_coalesce".  This function does the same thing
+** as the "coalesce" function.  This function also registers an SQL function
+** named "x_sqlite_exec" that invokes sqlite_exec().  Invoking sqlite_exec()
+** in this way is illegal recursion and should raise an SQLITE_MISUSE error.
+** The effect is similar to trying to use the same database connection from
+** two threads at the same time.
+**
+** The original motivation for this routine was to be able to call the
+** sqlite_create_function function while a query is in progress in order
+** to test the SQLITE_MISUSE detection logic.
+*/
+static int sqlite_test_create_function(
+  void *NotUsed,
+  Tcl_Interp *interp,    /* The TCL interpreter that invoked this command */
+  int argc,              /* Number of arguments */
+  char **argv            /* Text of each argument */
+){
+  sqlite *db;
+  extern void Md5_Register(sqlite*);
+  if( argc!=2 ){
+    Tcl_AppendResult(interp, "wrong # args: should be \"", argv[0],
+       " FILENAME\"", 0);
+    return TCL_ERROR;
+  }
+  db = (sqlite*)atoi(argv[1]);
+  sqlite_create_function(db, "x_coalesce", -1, ifnullFunc, 0);
+  sqlite_create_function(db, "x_sqlite_exec", 1, sqliteExecFunc, db);
+  return TCL_OK;
+}
+
+/*
+** Routines to implement the x_count() aggregate function.
+*/
+typedef struct CountCtx CountCtx;
+struct CountCtx {
+  int n;
+};
+static void countStep(sqlite_func *context, int argc, const char **argv){
+  CountCtx *p;
+  p = sqlite_aggregate_context(context, sizeof(*p));
+  if( (argc==0 || argv[0]) && p ){
+    p->n++;
+  }
+}   
+static void countFinalize(sqlite_func *context){
+  CountCtx *p;
+  p = sqlite_aggregate_context(context, sizeof(*p));
+  sqlite_set_result_int(context, p ? p->n : 0);
+}
+
+/*
+** Usage:  sqlite_test_create_aggregate DB
+**
+** Call the sqlite_create_function API on the given database in order
+** to create a function named "x_count".  This function does the same thing
+** as the "md5sum" function.
+**
+** The original motivation for this routine was to be able to call the
+** sqlite_create_aggregate function while a query is in progress in order
+** to test the SQLITE_MISUSE detection logic.
+*/
+static int sqlite_test_create_aggregate(
+  void *NotUsed,
+  Tcl_Interp *interp,    /* The TCL interpreter that invoked this command */
+  int argc,              /* Number of arguments */
+  char **argv            /* Text of each argument */
+){
+  sqlite *db;
+  if( argc!=2 ){
+    Tcl_AppendResult(interp, "wrong # args: should be \"", argv[0],
+       " FILENAME\"", 0);
+    return TCL_ERROR;
+  }
+  db = (sqlite*)atoi(argv[1]);
+  sqlite_create_aggregate(db, "x_count", 0, countStep, countFinalize, 0);
+  sqlite_create_aggregate(db, "x_count", 1, countStep, countFinalize, 0);
+  return TCL_OK;
+}
+
+
+
+/*
 ** Usage:  sqlite_mprintf_int FORMAT INTEGER INTEGER INTEGER
 **
 ** Call mprintf with three integer arguments
@@ -355,6 +466,10 @@ int Sqlitetest1_Init(Tcl_Interp *interp){
   Tcl_CreateCommand(interp, "sqlite_get_table_printf", test_get_table_printf,
       0, 0);
   Tcl_CreateCommand(interp, "sqlite_close", sqlite_test_close, 0, 0);
+  Tcl_CreateCommand(interp, "sqlite_create_function", 
+      sqlite_test_create_function, 0, 0);
+  Tcl_CreateCommand(interp, "sqlite_create_aggregate",
+      sqlite_test_create_aggregate, 0, 0);
   Tcl_LinkVar(interp, "sqlite_search_count", 
       (char*)&sqlite_search_count, TCL_LINK_INT);
 #ifdef MEMORY_DEBUG
diff --git a/src/util.c b/src/util.c
index 50cd8cfd2..a13b92af5 100644
--- a/src/util.c
+++ b/src/util.c
@@ -14,7 +14,7 @@
 ** This file contains functions for allocating memory, comparing
 ** strings, and stuff like that.
 **
-** $Id: util.c,v 1.42 2002/05/10 05:44:56 drh Exp $
+** $Id: util.c,v 1.43 2002/05/10 13:14:07 drh Exp $
 */
 #include "sqliteInt.h"
 #include <stdarg.h>
@@ -1069,35 +1069,57 @@ sqliteLikeCompare(const unsigned char *zPattern, const unsigned char *zString){
 }
 
 /*
-** Return a static string that describes the kind of error specified in the
-** argument.
+** Change the sqlite.magic from SQLITE_MAGIC_OPEN to SQLITE_MAGIC_BUSY.
+** Return an error (non-zero) if the magic was not SQLITE_MAGIC_OPEN
+** when this routine is called.
+**
+** This routine is a attempt to detect if two threads use the
+** same sqlite* pointer at the same time.  There is a race 
+** condition so it is possible that the error is not detected.
+** But usually the problem will be seen.  The result will be an
+** error which can be used to debugging the application that is
+** using SQLite incorrectly.
 */
-const char *sqlite_error_string(int rc){
-  const char *z;
-  switch( rc ){
-    case SQLITE_OK:         z = "not an error";                          break;
-    case SQLITE_ERROR:      z = "SQL logic error or missing database";   break;
-    case SQLITE_INTERNAL:   z = "internal SQLite implementation flaw";   break;
-    case SQLITE_PERM:       z = "access permission denied";              break;
-    case SQLITE_ABORT:      z = "callback requested query abort";        break;
-    case SQLITE_BUSY:       z = "database is locked";                    break;
-    case SQLITE_LOCKED:     z = "database table is locked";              break;
-    case SQLITE_NOMEM:      z = "out of memory";                         break;
-    case SQLITE_READONLY:   z = "attempt to write a readonly database";  break;
-    case SQLITE_INTERRUPT:  z = "interrupted";                           break;
-    case SQLITE_IOERR:      z = "disk I/O error";                        break;
-    case SQLITE_CORRUPT:    z = "database disk image is malformed";      break;
-    case SQLITE_NOTFOUND:   z = "table or record not found";             break;
-    case SQLITE_FULL:       z = "database is full";                      break;
-    case SQLITE_CANTOPEN:   z = "unable to open database file";          break;
-    case SQLITE_PROTOCOL:   z = "database locking protocol failure";     break;
-    case SQLITE_EMPTY:      z = "table contains no data";                break;
-    case SQLITE_SCHEMA:     z = "database schema has changed";           break;
-    case SQLITE_TOOBIG:     z = "too much data for one table row";       break;
-    case SQLITE_CONSTRAINT: z = "constraint failed";                     break;
-    case SQLITE_MISMATCH:   z = "datatype mismatch";                     break;
-    case SQLITE_MISUSE:     z = "SQLite library used incorrectly";       break;
-    default:                z = "unknown error";                         break;
+int sqliteSafetyOn(sqlite *db){
+  if( db->magic==SQLITE_MAGIC_OPEN ){
+    db->magic = SQLITE_MAGIC_BUSY;
+    return 0;
+  }else{
+    db->magic = SQLITE_MAGIC_ERROR;
+    db->flags |= SQLITE_Interrupt;
+    return 1;
+  }
+}
+
+/*
+** Change the magic from SQLITE_MAGIC_BUSY to SQLITE_MAGIC_OPEN.
+** Return an error (non-zero) if the magic was not SQLITE_MAGIC_BUSY
+** when this routine is called.
+*/
+int sqliteSafetyOff(sqlite *db){
+  if( db->magic==SQLITE_MAGIC_BUSY ){
+    db->magic = SQLITE_MAGIC_OPEN;
+    return 0;
+  }else{
+    db->magic = SQLITE_MAGIC_ERROR;
+    db->flags |= SQLITE_Interrupt;
+    return 1;
+  }
+}
+
+/*
+** Check to make sure we are not currently executing an sqlite_exec().
+** If we are currently in an sqlite_exec(), return true and set
+** sqlite.magic to SQLITE_MAGIC_ERROR.  This will cause a complete
+** shutdown of the database.
+**
+** This routine is used to try to detect when API routines are called
+** at the wrong time or in the wrong sequence.
+*/
+int sqliteSafetyCheck(sqlite *db){
+  if( db->recursionDepth ){
+    db->magic = SQLITE_MAGIC_ERROR;
+    return 1;
   }
-  return z;
+  return 0;
 }
diff --git a/src/vdbe.c b/src/vdbe.c
index 22b625c54..6d577f844 100644
--- a/src/vdbe.c
+++ b/src/vdbe.c
@@ -30,7 +30,7 @@
 ** But other routines are also provided to help in building up
 ** a program instruction by instruction.
 **
-** $Id: vdbe.c,v 1.140 2002/05/10 05:44:56 drh Exp $
+** $Id: vdbe.c,v 1.141 2002/05/10 13:14:07 drh Exp $
 */
 #include "sqliteInt.h"
 #include <ctype.h>
@@ -1112,8 +1112,12 @@ int sqliteVdbeList(
   for(i=0; rc==SQLITE_OK && i<p->nOp; i++){
     if( db->flags & SQLITE_Interrupt ){
       db->flags &= ~SQLITE_Interrupt;
-      sqliteSetString(pzErrMsg, "interrupted", 0);
-      rc = SQLITE_INTERRUPT;
+      if( db->magic!=SQLITE_MAGIC_BUSY ){
+        rc = SQLITE_MISUSE;
+      }else{
+        rc = SQLITE_INTERRUPT;
+      }
+      sqliteSetString(pzErrMsg, sqlite_error_string(rc), 0);
       break;
     }
     sprintf(zAddr,"%d",i);
@@ -1299,8 +1303,12 @@ int sqliteVdbeExec(
     */
     if( db->flags & SQLITE_Interrupt ){
       db->flags &= ~SQLITE_Interrupt;
-      rc = SQLITE_INTERRUPT;
-      sqliteSetString(pzErrMsg, "interrupted", 0);
+      if( db->magic!=SQLITE_MAGIC_BUSY ){
+        rc = SQLITE_MISUSE;
+      }else{
+        rc = SQLITE_INTERRUPT;
+      }
+      sqliteSetString(pzErrMsg, sqlite_error_string(rc), 0);
       break;
     }