From: danielk1977 <danielk1977@noemail.net>
Date: Fri, 16 Dec 2005 15:24:28 +0000 (+0000)
Subject: Verify that the rollback-hook is invoked correctly when a malloc() failure occurs... 
X-Git-Tag: version-3.6.10~3336
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f3f06bb30cb2d140c5ed6b79aee43bc497be4eb4;p=thirdparty%2Fsqlite.git

Verify that the rollback-hook is invoked correctly when a malloc() failure occurs. (CVS 2824)

FossilOrigin-Name: 83c8ae5bee3b6bdb556d2e85fa260ba855742601
---

diff --git a/manifest b/manifest
index 01a4d601e6..a846a709fe 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Add\sthe\ssqlite3_rollback_hook()\sAPI.\sStill\srequires\sfurther\stesting.\s(CVS\s2823)
-D 2005-12-16T06:54:02
+C Verify\sthat\sthe\srollback-hook\sis\sinvoked\scorrectly\swhen\sa\smalloc()\sfailure\soccurs.\s(CVS\s2824)
+D 2005-12-16T15:24:29
 F Makefile.in e3c6b3a38d734d41574c04f2fc90d18de2b87102
 F Makefile.linux-gcc aee18d8a05546dcf1888bd4547e442008a49a092
 F README 9c4e2d6706bdcc3efdd773ce752a8cdab4f90028
@@ -34,8 +34,8 @@ F src/alter.c 7bc737c689044fa00ff2f445139c61f5c9ee0ad8
 F src/analyze.c ea42005eed52c382fcc7ef66969e7f1858597633
 F src/attach.c ee70131f128d31a9c6dcb8824e8471c91b18601a
 F src/auth.c 31e2304bef67f44d635655f44234387ea7d21454
-F src/btree.c f880db4dc6339e279e982a342cbd1ff7c1f0d388
-F src/btree.h 1ed561263ca0e335bc3e81d761c9d5ff8c22f61e
+F src/btree.c 300fd8342f4d9f75829a8a96d60578592f3dee79
+F src/btree.h 8ff86378bb5af0cde282614c16bf0c0190b6d216
 F src/build.c 178a5c36365b5ec89a4a29a74186583f503c10fa
 F src/callback.c 62066afd516f220575e81b1a1239ab92a2eae252
 F src/complete.c df1681cef40dec33a286006981845f87b194e7a4
@@ -48,7 +48,7 @@ F src/hash.c 8747cf51d12de46512880dfcf1b68b4e24072863
 F src/hash.h 1b0c445e1c89ff2aaad9b4605ba61375af001e84
 F src/insert.c a5629e462560d6aa9be7e0dafb6ed1518bb623bb
 F src/legacy.c 59757d857ab95fcbb0ac27692d3201e35f093dd7
-F src/main.c cbab4a1b056eddf03f671f0e9d76f69f48e6bff2
+F src/main.c ec04b37605752a4294ae98f2b7d78ba62950ba2f
 F src/md5.c c5fdfa5c2593eaee2e32a5ce6c6927c986eaf217
 F src/os.c 7b4a002d9c9421580276db55d2329636a604e8ef
 F src/os.h e941992043b127fdb1bd114f0b4319ae1c4562a7
@@ -68,7 +68,7 @@ F src/printf.c f47a2f4b5387cd2ebb12e9117a1a5d6bd9a2b812
 F src/random.c ff5e9a8cad790e2a51cd4d2e7737dc8540e09d1d
 F src/select.c 2292b065bc6be61e01aad39a2e1b93e332fb7e57
 F src/shell.c 4872acee1d2a826c73c914961e469e563204b7f9
-F src/sqlite.h.in 6b9ec2fee44d5a2da19e72f2890f802a9efe8e7c
+F src/sqlite.h.in 184143f88471cdd4690ae9304ba134a48d10dae1
 F src/sqliteInt.h e6063bc65e02682ee923f795b74afa85d9823f9a
 F src/table.c 486dcfce532685b53b5a2b5da8bba0ded6fb2316
 F src/tclsqlite.c b21dd96ffab1625c986513e003a1945a6ae112ae
@@ -84,11 +84,11 @@ F src/update.c ec8e540617b116725b5a55c8d6b4db8bc67fdd7d
 F src/utf.c b7bffac4260177ae7f83c01d025fe0f5ed70ce71
 F src/util.c 8bb5e0553692d36c769d6cd033eb7f68a7586648
 F src/vacuum.c fbfdd3967fd34e2f260fafed88dcbf3c10856b94
-F src/vdbe.c a9acffc91d3e06af24dc1dcf5bf7d5e77460b1c5
+F src/vdbe.c 09aaed71f076bfd4286607ee4845100b910a492f
 F src/vdbe.h 8729a4ee16ff9aeab2af9667df3cf300ff978e13
 F src/vdbeInt.h 7b8b8c5dcb203243e4a9a4414c9e488473f67741
 F src/vdbeapi.c b270b680cbc5d20b5a1abfdb08339667985df94e
-F src/vdbeaux.c c878807fd0f5829c9a02fa069db2ec69692b0ff6
+F src/vdbeaux.c c38276e0c95d9ed9c35bab195faea3d57e787aea
 F src/vdbefifo.c 9efb94c8c3f4c979ebd0028219483f88e57584f5
 F src/vdbemem.c deba8d6e3727643924b210a8c531a496c2b8d386
 F src/where.c 269569f380ddc018518f67765fe2f0d3c8760e28
@@ -153,7 +153,7 @@ F test/enc3.test f6a5f0b7b7f3a88f030d3143729b87cd5c86d837
 F test/expr.test 06381174d8c25fbbfd9ed335fe0cde835d39123d
 F test/fkey1.test 153004438d51e6769fb1ce165f6313972d6263ce
 F test/func.test e891bebf5a939fd45c9453ad35e2280f41a30687
-F test/hook.test 2b771652ae4f27e95fc3e6992c618af7954ce785
+F test/hook.test 3bae7892e04ba40f20e205027c3ba343e46d22ff
 F test/in.test cead6165aebbe0d451bb2263a307173acfeb6240
 F test/index.test 3871c47ec475f779f0b99dc36a3d177951995712
 F test/index2.test 9ad98243fd7fe833795a9cc662f371f0eed4ff4f
@@ -180,7 +180,7 @@ F test/lock3.test 615111293cf32aa2ed16d01c6611737651c96fb9
 F test/main.test b12f01d49a5c805a33fa6c0ef168691f63056e79
 F test/malloc.test a5ed721cf7d1b12602ede4f98c11b65ab1582cc0
 F test/malloc2.test e6e321db96d6c94cb18bf82ad7215070c41e624e
-F test/malloc3.test 56372aafe58cc24a31bad4b24ff3f77cf1cc30ab
+F test/malloc3.test 9797d39eca7087a1022bcc0eb0b22215475c9698
 F test/malloc4.test 2e29d155eb4b4808019ef47eeedfcbe9e09e0f05
 F test/manydb.test d81debbf5871242e3b5df1d3bb5e14c50431b6f8
 F test/memdb.test 1860e060be810bf0775bc57408a5b7c4954bcaea
@@ -327,7 +327,7 @@ F www/tclsqlite.tcl ddcf912ea48695603c8ed7efb29f0812ef8d1b49
 F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0
 F www/version3.tcl a99cf5f6d8bd4d5537584a2b342f0fb9fa601d8b
 F www/whentouse.tcl 97e2b5cd296f7d8057e11f44427dea8a4c2db513
-P cd110aa225b09591064405dd8952b3df37278c52
-R 3161ff073c4e4c2688e57ee8d03a85ab
+P 3baa3ff32435b64e7ae7646b17a98fef9296aaa0
+R 7c3f6fd17e2d44aaf5ad837b01c26c23
 U danielk1977
-Z 7173f459f67aead630a0b046899d180b
+Z 06f497bf2cbb4256c1ee4b87f97222aa
diff --git a/manifest.uuid b/manifest.uuid
index d22ff49843..8c8443dd54 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-3baa3ff32435b64e7ae7646b17a98fef9296aaa0
\ No newline at end of file
+83c8ae5bee3b6bdb556d2e85fa260ba855742601
\ No newline at end of file
diff --git a/src/btree.c b/src/btree.c
index 65b5594dd6..052f917b7d 100644
--- a/src/btree.c
+++ b/src/btree.c
@@ -9,7 +9,7 @@
 **    May you share freely, never taking more than you give.
 **
 *************************************************************************
-** $Id: btree.c,v 1.274 2005/12/16 06:54:02 danielk1977 Exp $
+** $Id: btree.c,v 1.275 2005/12/16 15:24:29 danielk1977 Exp $
 **
 ** This file implements a external (disk-based) database using BTrees.
 ** For a detailed discussion of BTrees, refer to
@@ -5818,15 +5818,3 @@ int sqlite3BtreeSync(Btree *pBt, const char *zMaster){
   return SQLITE_OK;
 }
 
-#ifndef SQLITE_OMIT_GLOBALRECOVER
-/*
-** Reset the btree and underlying pager after a malloc() failure. Any
-** transaction that was active when malloc() failed is rolled back.
-*/
-int sqlite3BtreeReset(Btree *pBt){
-  if( pBt->pCursor ) return SQLITE_BUSY;
-  pBt->inTrans = TRANS_NONE;
-  unlockBtreeIfUnused(pBt);
-  return sqlite3pager_reset(pBt->pPager);
-}
-#endif
diff --git a/src/btree.h b/src/btree.h
index acc29710f3..61c1cc3d57 100644
--- a/src/btree.h
+++ b/src/btree.h
@@ -13,7 +13,7 @@
 ** subsystem.  See comments in the source code for a detailed description
 ** of what each interface routine does.
 **
-** @(#) $Id: btree.h,v 1.64 2005/08/27 16:36:49 drh Exp $
+** @(#) $Id: btree.h,v 1.65 2005/12/16 15:24:29 danielk1977 Exp $
 */
 #ifndef _BTREE_H_
 #define _BTREE_H_
@@ -74,7 +74,6 @@ int sqlite3BtreeCreateTable(Btree*, int*, int flags);
 int sqlite3BtreeIsInTrans(Btree*);
 int sqlite3BtreeIsInStmt(Btree*);
 int sqlite3BtreeSync(Btree*, const char *zMaster);
-int sqlite3BtreeReset(Btree *);
 
 const char *sqlite3BtreeGetFilename(Btree *);
 const char *sqlite3BtreeGetDirname(Btree *);
diff --git a/src/main.c b/src/main.c
index ba0bb5feb3..2315a6df0d 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.311 2005/12/16 06:54:02 danielk1977 Exp $
+** $Id: main.c,v 1.312 2005/12/16 15:24:29 danielk1977 Exp $
 */
 #include "sqliteInt.h"
 #include "os.h"
@@ -204,8 +204,12 @@ int sqlite3_close(sqlite3 *db){
 */
 void sqlite3RollbackAll(sqlite3 *db){
   int i;
+  int inTrans = 0;
   for(i=0; i<db->nDb; i++){
     if( db->aDb[i].pBt ){
+      if( sqlite3BtreeIsInTrans(db->aDb[i].pBt) ){
+        inTrans = 1;
+      }
       sqlite3BtreeRollback(db->aDb[i].pBt);
       db->aDb[i].inTrans = 0;
     }
@@ -215,7 +219,7 @@ void sqlite3RollbackAll(sqlite3 *db){
   }
 
   /* If one has been configured, invoke the rollback-hook callback */
-  if( db->xRollbackCallback ){
+  if( db->xRollbackCallback && (inTrans || !db->autoCommit) ){
     db->xRollbackCallback(db->pRollbackArg);
   }
 }
diff --git a/src/sqlite.h.in b/src/sqlite.h.in
index 966b38a206..551fddc23b 100644
--- a/src/sqlite.h.in
+++ b/src/sqlite.h.in
@@ -12,7 +12,7 @@
 ** This header file defines the interface that the SQLite library
 ** presents to client programs.
 **
-** @(#) $Id: sqlite.h.in,v 1.147 2005/12/16 06:54:02 danielk1977 Exp $
+** @(#) $Id: sqlite.h.in,v 1.148 2005/12/16 15:24:29 danielk1977 Exp $
 */
 #ifndef _SQLITE3_H_
 #define _SQLITE3_H_
@@ -1319,6 +1319,21 @@ void *sqlite3_update_hook(
   void*
 );
 
+/*
+** Register a callback to be invoked whenever a transaction is rolled
+** back. 
+**
+** The new callback function overrides any existing rollback-hook
+** callback. If there was an existing callback, then it's pArg value 
+** (the third argument to sqlite3_rollback_hook() when it was registered) 
+** is returned. Otherwise, NULL is returned.
+**
+** For the purposes of this API, a transaction is said to have been 
+** rolled back if an explicit "ROLLBACK" statement is executed, or
+** an error or constraint causes an implicit rollback to occur. The 
+** callback is not invoked if a transaction is automatically rolled
+** back because the database connection is closed.
+*/
 void *sqlite3_rollback_hook(sqlite3*, void(*)(void *), void*);
 
 /*
diff --git a/src/vdbe.c b/src/vdbe.c
index 95e024d206..4cedcd5c2a 100644
--- a/src/vdbe.c
+++ b/src/vdbe.c
@@ -43,7 +43,7 @@
 ** in this file for details.  If in doubt, do not deviate from existing
 ** commenting and indentation practices when changing or adding code.
 **
-** $Id: vdbe.c,v 1.505 2005/12/15 15:22:10 danielk1977 Exp $
+** $Id: vdbe.c,v 1.506 2005/12/16 15:24:29 danielk1977 Exp $
 */
 #include "sqliteInt.h"
 #include "os.h"
@@ -2299,16 +2299,19 @@ case OP_AutoCommit: {       /* no-push */
         " transaction - SQL statements in progress", 0);
     rc = SQLITE_ERROR;
   }else if( i!=db->autoCommit ){
-    db->autoCommit = i;
     if( pOp->p2 ){
       assert( i==1 );
       sqlite3RollbackAll(db);
-    }else if( sqlite3VdbeHalt(p)==SQLITE_BUSY ){
-      p->pTos = pTos;
-      p->pc = pc;
-      db->autoCommit = 1-i;
-      p->rc = SQLITE_BUSY;
-      return SQLITE_BUSY;
+      db->autoCommit = 1;
+    }else{
+      db->autoCommit = i;
+      if( sqlite3VdbeHalt(p)==SQLITE_BUSY ){
+        p->pTos = pTos;
+        p->pc = pc;
+        db->autoCommit = 1-i;
+        p->rc = SQLITE_BUSY;
+        return SQLITE_BUSY;
+      }
     }
     return SQLITE_DONE;
   }else{
diff --git a/src/vdbeaux.c b/src/vdbeaux.c
index a99dc46434..82977e8810 100644
--- a/src/vdbeaux.c
+++ b/src/vdbeaux.c
@@ -1177,12 +1177,12 @@ int sqlite3VdbeHalt(Vdbe *p){
         return SQLITE_BUSY;
       }else if( rc!=SQLITE_OK ){
         p->rc = rc;
-        xFunc = sqlite3BtreeRollback;
+        sqlite3RollbackAll(db);
       }else{
         sqlite3CommitInternalChanges(db);
       }
     }else{
-      xFunc = sqlite3BtreeRollback;
+      sqlite3RollbackAll(db);
     }
   }else{
 
@@ -1227,9 +1227,9 @@ int sqlite3VdbeHalt(Vdbe *p){
     }else if( p->errorAction==OE_Abort ){
       xFunc = sqlite3BtreeRollbackStmt;
     }else{
-      xFunc = sqlite3BtreeRollback;
-      db->autoCommit = 1;
       abortOtherActiveVdbes(p);
+      sqlite3RollbackAll(db);
+      db->autoCommit = 1;
     }
   }
 
@@ -1240,20 +1240,14 @@ int sqlite3VdbeHalt(Vdbe *p){
   */
   assert(!xFunc ||
     xFunc==sqlite3BtreeCommitStmt ||
-    xFunc==sqlite3BtreeRollbackStmt ||
-    xFunc==sqlite3BtreeRollback
+    xFunc==sqlite3BtreeRollbackStmt
   );
-  if( xFunc==sqlite3BtreeRollback ){
-    assert( p->rc!=SQLITE_OK );
-    sqlite3RollbackAll(db);
-  }else{
-    for(i=0; xFunc && i<db->nDb; i++){ 
-      int rc;
-      Btree *pBt = db->aDb[i].pBt;
-      if( pBt ){
-        rc = xFunc(pBt);
-        if( p->rc==SQLITE_OK ) p->rc = rc;
-      }
+  for(i=0; xFunc && i<db->nDb; i++){ 
+    int rc;
+    Btree *pBt = db->aDb[i].pBt;
+    if( pBt ){
+      rc = xFunc(pBt);
+      if( p->rc==SQLITE_OK ) p->rc = rc;
     }
   }
 
@@ -1270,9 +1264,7 @@ int sqlite3VdbeHalt(Vdbe *p){
   /* Rollback or commit any schema changes that occurred. */
   if( p->rc!=SQLITE_OK && db->flags&SQLITE_InternChanges ){
     sqlite3ResetInternalSchema(db, 0);
-    if( xFunc!=sqlite3BtreeRollback ){
-      db->flags = (db->flags | SQLITE_InternChanges);
-    }
+    db->flags = (db->flags | SQLITE_InternChanges);
   }
 
   /* We have successfully halted and closed the VM.  Record this fact. */
diff --git a/test/hook.test b/test/hook.test
index dcffd7053c..0d443e78aa 100644
--- a/test/hook.test
+++ b/test/hook.test
@@ -17,7 +17,7 @@
 #      sqlite_update_hook    (tests hook-4-*)
 #      sqlite_rollback_hook  (tests hook-5.*)
 #
-# $Id: hook.test,v 1.7 2005/12/16 06:54:03 danielk1977 Exp $
+# $Id: hook.test,v 1.8 2005/12/16 15:24:30 danielk1977 Exp $
 
 set testdir [file dirname $argv0]
 source $testdir/tester.tcl
@@ -231,8 +231,12 @@ db update_hook {}
 # error condition).
 #
 # hook-5.1.* - Test explicit rollbacks.
-# hook-5.2.* - Test implicit rollbacks.
-# hook-5.3.* - Test hot-journal rollbacks.
+# hook-5.2.* - Test implicit rollbacks caused by constraint failure.
+#
+# hook-5.3.* - Test implicit rollbacks caused by IO errors.
+# hook-5.4.* - Test implicit rollbacks caused by malloc() failure.
+# hook-5.5.* - Test hot-journal rollbacks. Or should the rollback hook 
+#              not be called for these?
 #
 
 do_test hook-5.0 {
@@ -243,6 +247,7 @@ do_test hook-5.0 {
 } {}
 
 # Test explicit rollbacks. Not much can really go wrong here.
+#
 do_test hook-5.1.1 {
   set ::rollback_hook 0
   execsql {
@@ -253,6 +258,7 @@ do_test hook-5.1.1 {
 } {1}
 
 # Test implicit rollbacks caused by constraints.
+#
 do_test hook-5.2.1 {
   set ::rollback_hook 0
   catchsql {
@@ -271,6 +277,7 @@ do_test hook-5.2.2 {
 } {1}
 
 #
+# End rollback-hook testing.
 #----------------------------------------------------------------------------
 
 finish_test
diff --git a/test/malloc3.test b/test/malloc3.test
index c1f13652b2..f70e1cff6f 100644
--- a/test/malloc3.test
+++ b/test/malloc3.test
@@ -13,7 +13,7 @@
 # correctly. The emphasis of these tests are the _prepare(), _step() and
 # _finalize() calls.
 #
-# $Id: malloc3.test,v 1.4 2005/12/09 14:25:12 danielk1977 Exp $
+# $Id: malloc3.test,v 1.5 2005/12/16 15:24:30 danielk1977 Exp $
 
 set testdir [file dirname $argv0]
 source $testdir/tester.tcl
@@ -503,6 +503,8 @@ proc run_test {arglist {pcstart 0} {iFailStart 1}} {
     if {$ac && !$nac} {set begin_pc $i}
   }
 
+  db rollback_hook [list incr ::rollback_hook_count]
+
   set iFail $iFailStart
   set pc $pcstart
   while {$pc*2 < [llength $arglist]} {
@@ -523,6 +525,8 @@ proc run_test {arglist {pcstart 0} {iFailStart 1}} {
       }
 
       -sql {
+        set ::rollback_hook_count 0
+
         set ac [sqlite3_get_autocommit $::DB]        ;# Auto-Commit
         sqlite_malloc_fail $iFail
 # puts "SQL $iterid [lindex $v 1]"
@@ -530,14 +534,32 @@ proc run_test {arglist {pcstart 0} {iFailStart 1}} {
 # puts "rc = $rc msg = \"$msg\""
         set nac [sqlite3_get_autocommit $::DB]       ;# New Auto-Commit 
 
+
+        if {$rc != 0 && $nac && !$ac} {
+          # Before [db eval] the auto-commit flag was clear. Now it
+          # is set. Since an error occured we assume this was not a
+	  # commit - therefore a rollback occured. Check that the
+	  # rollback-hook was invoked.
+          do_test malloc3-rollback_hook.$iterid {
+            set ::rollback_hook_count
+          } {1}
+        }
+
         if {$rc == 0} {
-            # Successful execution of sql.
+            # Successful execution of sql. Our "mallocs-until-failure" 
+            # count should be greater than 0. Otherwise a malloc() failed
+            # and the error was not reported.
             if {[lindex [sqlite_malloc_stat] 2] <= 0} {
               error "Unreported malloc() failure"
             }
+
             if {$ac && !$nac} {
+              # Before the [db eval] the auto-commit flag was set, now it
+              # is clear. We can deduce that a "BEGIN" statement has just
+              # been successfully executed.
               set begin_pc $pc
             } 
+
             incr pc
             set iFail 1
             sqlite_malloc_fail 0
@@ -547,9 +569,11 @@ proc run_test {arglist {pcstart 0} {iFailStart 1}} {
             integrity_check "malloc3-(integrity).$iterid"
             incr iFail
             if {$nac && !$ac} {
+
               if {![lindex $v 0]} {
                 error "Statement \"[lindex $v 1]\" caused a rollback"
               }
+
 # puts "Statement \"[lindex $v 1]\" caused a rollback"
               for {set i $begin_pc} {$i < $pc} {incr i} {
                 set k2 [lindex $arglist [expr 2 * $i]]
@@ -587,7 +611,7 @@ proc run_test {arglist {pcstart 0} {iFailStart 1}} {
 # Turn of the Tcl interface's prepared statement caching facility.
 db cache size 0
 
-run_test $::run_test_script 
+run_test $::run_test_script
 # run_test [lrange $::run_test_script 0 3] 0 63
 sqlite_malloc_fail 0
 db close