Fix a problem causing the BtShared.isPending flag to be cleared to early. Also coverage improvements for btree.c. (CVS 6440)

FossilOrigin-Name: 8f1423445b29a5f52ed907de6db82128a96ebfe2

5 files changed, 152 insertions, 18 deletions

diff --git a/manifest b/manifest
index f77497653..ca77fb3bd 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Ensure\sthe\srequired\sVerifyCookie/Transaction/TableLock\sopcodes\sare\sadded\sfor\s"x\nIN\s(SELECT\sc\sFROM\st)"\sexpressions.\sTicket\s#3771.\s(CVS\s6439)
-D 2009-04-02T17:23:33
+C Fix\sa\sproblem\scausing\sthe\sBtShared.isPending\sflag\sto\sbe\scleared\sto\searly.\sAlso\scoverage\simprovements\sfor\sbtree.c.\s(CVS\s6440)
+D 2009-04-02T18:28:08
 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0
 F Makefile.in 583e87706abc3026960ed759aff6371faf84c211
 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654
@@ -103,7 +103,7 @@ F src/auth.c c8b2ab5c8bad4bd90ed7c294694f48269162c627
 F src/backup.c 0082d0e5a63f04e88faee0dff0a7d63d3e92a78d
 F src/bitvec.c ef370407e03440b0852d05024fb016b14a471d3d
 F src/btmutex.c 341502bc496dc0840dcb00cde65680fb0e85c3ab
-F src/btree.c dcf8157bd4b28a6125d2900f3031a034c1f57720
+F src/btree.c 7d51b2ef5c09ebc05bce9a1a1f2f5b9e1b93d9e0
 F src/btree.h e302c5747494067cd4f5763000fbe7bca767d816
 F src/btreeInt.h df64030d632f8c8ac217ed52e8b6b3eacacb33a5
 F src/build.c 72357fd75ef036d0afbf1756edab6d62c56fcf4b
@@ -276,7 +276,7 @@ F test/colmeta.test 087c42997754b8c648819832241daf724f813322
 F test/colname.test 69fea30632d55862a012d3786c7d5fd1329f78db
 F test/conflict.test bb29b052c60a1f7eb6382be77902061d1f305318
 F test/corrupt.test 5bcf7a986358123b8055dfa64b45fc2fb54dcaa9
-F test/corrupt2.test b83f690753525ddac4a9c96e6ba11886ddaf24ea
+F test/corrupt2.test 465786cf3fb286b6b674c6342388dfff988f950b
 F test/corrupt3.test 263e8bb04e2728df832fddf6973cf54c91db0c32
 F test/corrupt4.test acdb01afaedf529004b70e55de1a6f5a05ae7fff
 F test/corrupt5.test 7796d5bdfe155ed824cee9dff371f49da237cfe0
@@ -532,7 +532,7 @@ F test/shared.test 3b448dc0f7a9356e641894ed81c27599f39d809d
 F test/shared2.test 8973e41e008acbbd8d1b191b91a23cd472f058e8
 F test/shared3.test 9c880afc081d797da514ef64bccf36f3fce2f09c
 F test/shared4.test d0fadacb50bb6981b2fb9dc6d1da30fa1edddf83
-F test/shared6.test d3e0db3cbfb0d64933d8a9016f7eaa40f15a60ac
+F test/shared6.test db3906045175debea4a0d57cb94c6ffe2d85413d
 F test/shared_err.test 91e26ec4f3fbe07951967955585137e2f18993de
 F test/shortread1.test bb591ef20f0fd9ed26d0d12e80eee6d7ac8897a3
 F test/sidedelete.test f0ad71abe6233e3b153100f3b8d679b19a488329
@@ -715,7 +715,7 @@ F tool/speedtest16.c c8a9c793df96db7e4933f0852abb7a03d48f2e81
 F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff
 F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224
 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e
-P 53dac0a455b9a822f710c257711e8d319060cf84
-R 4a174564884f326a4622b10ba400e069
+P 058a2f20930d7707c03c3c27db8e761d5657ee46
+R 3a4050ae67f1e8a6ec248bda7bba7d51
 U danielk1977
-Z 805670f748f6b920a00761168c563c97
+Z dc4c28528723be8812cc90fabf0cbd80
diff --git a/manifest.uuid b/manifest.uuid
index fe292cdb1..64cdd9b09 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-058a2f20930d7707c03c3c27db8e761d5657ee46
\ No newline at end of file
+8f1423445b29a5f52ed907de6db82128a96ebfe2
\ No newline at end of file
diff --git a/src/btree.c b/src/btree.c
index 69264f42a..09412080f 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.587 2009/04/01 19:07:04 danielk1977 Exp $
+** $Id: btree.c,v 1.588 2009/04/02 18:28:08 danielk1977 Exp $
 **
 ** This file implements a external (disk-based) database using BTrees.
 ** See the header comment on "btreeInt.h" for additional information.
@@ -248,6 +248,10 @@ static int setSharedCacheTableLock(Btree *p, Pgno iTable, u8 eLock){
 /*
 ** Release all the table locks (locks obtained via calls to
 ** the setSharedCacheTableLock() procedure) held by Btree handle p.
+**
+** This function assumes that handle p has an open read or write 
+** transaction. If it does not, then the BtShared.isPending variable
+** may be incorrectly cleared.
 */
 static void clearAllSharedCacheTableLocks(Btree *p){
   BtShared *pBt = p->pBt;
@@ -255,10 +259,12 @@ static void clearAllSharedCacheTableLocks(Btree *p){
 
   assert( sqlite3BtreeHoldsMutex(p) );
   assert( p->sharable || 0==*ppIter );
+  assert( p->inTrans>0 );
 
   while( *ppIter ){
     BtLock *pLock = *ppIter;
     assert( pBt->isExclusive==0 || pBt->pWriter==pLock->pBtree );
+    assert( pLock->pBtree->inTrans>=pLock->eLock );
     if( pLock->pBtree==p ){
       *ppIter = pLock->pNext;
       sqlite3_free(pLock);
@@ -2261,7 +2267,7 @@ set_child_ptrmaps_out:
 }
 
 /*
-** Somewhere on pPage, which is guarenteed to be a btree page, not an overflow
+** Somewhere on pPage, which is guaranteed to be a btree page, not an overflow
 ** page, is a pointer to page iFrom. Modify this pointer so that it points to
 ** iTo. Parameter eType describes the type of pointer to be modified, as 
 ** follows:
@@ -2424,6 +2430,7 @@ static int incrVacuumStep(BtShared *pBt, Pgno nFin, Pgno iLastPg){
   Pgno nFreeList;           /* Number of pages still on the free-list */
 
   assert( sqlite3_mutex_held(pBt->mutex) );
+  assert( iLastPg>nFin );
 
   if( !PTRMAP_ISPAGE(pBt, iLastPg) && iLastPg!=PENDING_BYTE_PAGE(pBt) ){
     int rc;
@@ -2431,7 +2438,7 @@ static int incrVacuumStep(BtShared *pBt, Pgno nFin, Pgno iLastPg){
     Pgno iPtrPage;
 
     nFreeList = get4byte(&pBt->pPage1->aData[36]);
-    if( nFreeList==0 || nFin==iLastPg ){
+    if( nFreeList==0 ){
       return SQLITE_DONE;
     }
 
@@ -2688,7 +2695,6 @@ int sqlite3BtreeCommitPhaseTwo(Btree *p){
     }
     pBt->inTransaction = TRANS_READ;
   }
-  clearAllSharedCacheTableLocks(p);
 
   /* If the handle has any kind of transaction open, decrement the transaction
   ** count of the shared btree. If the transaction count reaches 0, set
@@ -2696,6 +2702,7 @@ int sqlite3BtreeCommitPhaseTwo(Btree *p){
   ** will unlock the pager.
   */
   if( p->inTrans!=TRANS_NONE ){
+    clearAllSharedCacheTableLocks(p);
     pBt->nTransaction--;
     if( 0==pBt->nTransaction ){
       pBt->inTransaction = TRANS_NONE;
@@ -2812,7 +2819,6 @@ int sqlite3BtreeRollback(Btree *p){
   }
 #endif
   btreeIntegrity(p);
-  clearAllSharedCacheTableLocks(p);
 
   if( p->inTrans==TRANS_WRITE ){
     int rc2;
@@ -2834,6 +2840,7 @@ int sqlite3BtreeRollback(Btree *p){
   }
 
   if( p->inTrans!=TRANS_NONE ){
+    clearAllSharedCacheTableLocks(p);
     assert( pBt->nTransaction>0 );
     pBt->nTransaction--;
     if( 0==pBt->nTransaction ){
@@ -6788,8 +6795,14 @@ int sqlite3BtreeGetMeta(Btree *p, int idx, u32 *pMeta){
   if( idx==4 && *pMeta>0 ) pBt->readOnly = 1;
 #endif
 
-  /* Grab the read-lock on page 1. */
-  rc = setSharedCacheTableLock(p, 1, READ_LOCK);
+  /* If there is currently an open transaction, grab a read-lock 
+  ** on page 1 of the database file. This is done to make sure that
+  ** no other connection can modify the meta value just read from
+  ** the database until the transaction is concluded.
+  */
+  if( p->inTrans>0 ){
+    rc = setSharedCacheTableLock(p, 1, READ_LOCK);
+  }
   sqlite3BtreeLeave(p);
   return rc;
 }
diff --git a/test/corrupt2.test b/test/corrupt2.test
index 652f7844a..3f5019623 100644
--- a/test/corrupt2.test
+++ b/test/corrupt2.test
@@ -13,7 +13,7 @@
 # This file implements tests to make sure SQLite does not crash or
 # segfault if it sees a corrupt database file.
 #
-# $Id: corrupt2.test,v 1.18 2008/09/29 11:49:48 danielk1977 Exp $
+# $Id: corrupt2.test,v 1.19 2009/04/02 18:28:08 danielk1977 Exp $
 
 set testdir [file dirname $argv0]
 source $testdir/tester.tcl
@@ -455,4 +455,48 @@ corruption_test -sqlprep {
   } {SQLITE_CORRUPT}
 }
 
+corruption_test -sqlprep {
+  PRAGMA auto_vacuum = incremental;
+  CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+  CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+  INSERT INTO t1 VALUES(1, randstr(100,100));
+  INSERT INTO t1 SELECT NULL, randstr(100,100) FROM t1;
+  INSERT INTO t1 SELECT NULL, randstr(100,100) FROM t1;
+  INSERT INTO t1 SELECT NULL, randstr(100,100) FROM t1;
+  INSERT INTO t1 SELECT NULL, randstr(100,100) FROM t1;
+  INSERT INTO t1 SELECT NULL, randstr(100,100) FROM t1;
+  INSERT INTO t2 SELECT * FROM t1;
+  DELETE FROM t1;
+} -corrupt {
+  set offset [expr [file size corrupt.db] - 1024]
+  hexio_write corrupt.db $offset FF 
+  hexio_write corrupt.db 24   12345678
+} -test {
+  do_test corrupt2-11.1 {
+    catchsql { PRAGMA incremental_vacuum }
+  } {1 {database disk image is malformed}}
+}
+
+corruption_test -sqlprep {
+  PRAGMA auto_vacuum = incremental;
+  CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+  CREATE TABLE t2(a INTEGER PRIMARY KEY, b);
+  INSERT INTO t1 VALUES(1, randstr(100,100));
+  INSERT INTO t1 SELECT NULL, randstr(100,100) FROM t1;
+  INSERT INTO t1 SELECT NULL, randstr(100,100) FROM t1;
+  INSERT INTO t1 SELECT NULL, randstr(100,100) FROM t1;
+  INSERT INTO t1 SELECT NULL, randstr(100,100) FROM t1;
+  INSERT INTO t1 SELECT NULL, randstr(100,100) FROM t1;
+  INSERT INTO t2 SELECT * FROM t1;
+  DELETE FROM t1;
+} -corrupt {
+  set pgno [expr [file size corrupt.db] / 1024]
+  hexio_write corrupt.db [expr 1024+5*($pgno-3)] 03 
+  hexio_write corrupt.db 24   12345678
+} -test {
+  do_test corrupt2-12.1 {
+    catchsql { PRAGMA incremental_vacuum }
+  } {1 {database disk image is malformed}}
+}
+
 finish_test
diff --git a/test/shared6.test b/test/shared6.test
index 2d22a301a..ef04aac85 100644
--- a/test/shared6.test
+++ b/test/shared6.test
@@ -9,7 +9,7 @@
 #
 #***********************************************************************
 #
-# $Id: shared6.test,v 1.2 2009/04/01 18:25:54 danielk1977 Exp $
+# $Id: shared6.test,v 1.3 2009/04/02 18:28:08 danielk1977 Exp $
 
 set testdir [file dirname $argv0]
 source $testdir/tester.tcl
@@ -21,6 +21,7 @@ do_test shared6-1.1.1 {
     CREATE TABLE t2(c, d);
     CREATE TABLE t3(e, f);
   }
+  db close
 } {}
 do_test shared6-1.1.2 {
   set ::enable_shared_cache [sqlite3_enable_shared_cache 1]
@@ -133,9 +134,11 @@ do_test shared6-1.X {
   db2 close
 } {}
 
+#-------------------------------------------------------------------------
 # The following tests - shared6-2.* - test that two database connections
 # that connect to the same file using different VFS implementations do
 # not share a cache.
+#
 if {$::tcl_platform(platform) eq "unix"} {
   do_test shared6-2.1 {
     sqlite3 db1 test.db -vfs unix
@@ -174,6 +177,80 @@ if {$::tcl_platform(platform) eq "unix"} {
   } {}
 }
 
+#-------------------------------------------------------------------------
+# Test that it is possible to open an exclusive transaction while 
+# already holding a read-lock on the database file. And that it is
+# not possible if some other connection holds such a lock.
+#
+do_test shared6-3.1 {
+  sqlite3 db1 test.db
+  sqlite3 db2 test.db
+  sqlite3 db3 test.db
+} {}
+db1 eval {SELECT * FROM t1} {
+  # Within this block [db1] is holding a read-lock on t1. Test that
+  # this means t1 cannot be written by [db2].
+  #
+  do_test shared6-3.2 {
+    catchsql { INSERT INTO t1 VALUES(1, 2) } db2
+  } {1 {database table is locked: t1}}
+
+  do_test shared6-3.3 {
+    execsql { BEGIN EXCLUSIVE } db1
+  } {}
+  break
+}
+do_test shared6-3.4 {
+  catchsql { SELECT * FROM t1 } db2
+} {1 {database schema is locked: main}}
+do_test shared6-3.5 {
+  execsql COMMIT db1
+} {}
+db2 eval {SELECT * FROM t1} {
+  do_test shared6-3.6 {
+    catchsql { BEGIN EXCLUSIVE } db1
+  } {1 {database table is locked}}
+  break
+}
+do_test shared6-3.7 {
+  execsql { BEGIN } db1
+  execsql { BEGIN } db2
+} {}
+db2 eval {SELECT * FROM t1} {
+  do_test shared6-3.8 {
+    catchsql { INSERT INTO t1 VALUES(1, 2) } db1
+  } {1 {database table is locked: t1}}
+  break
+}
+do_test shared6-3.9 {
+  execsql { BEGIN ; ROLLBACK } db3
+} {}
+do_test shared6-3.10 {
+  catchsql { SELECT * FROM t1 } db3
+} {1 {database table is locked}}
+do_test shared6-3.X {
+  db1 close
+  db2 close
+  db3 close
+} {}
+
+do_test shared6-4.1 {
+  #file delete -force test.db test.db-journal
+  sqlite3 db1 test.db
+  sqlite3 db2 test.db
+
+  set ::STMT [sqlite3_prepare_v2 db1 "SELECT * FROM t1" -1 DUMMY]
+  execsql { CREATE TABLE t5(a, b) } db2
+} {}
+do_test shared6-4.2 {
+  sqlite3_finalize $::STMT
+} {SQLITE_OK}
+do_test shared6-4.X {
+  
+  db1 close
+  db2 close
+} {}
+
 sqlite3_enable_shared_cache $::enable_shared_cache
 finish_test