Invalidate sqlite3_blob* handles whenever an SQL statement is used to delete or modify the rows containing the open blob. Previously, modifying the table containing the open blob in any way invalidated the handle. This was too restrictive. (CVS 5199)

FossilOrigin-Name: e339c91f8718482ce74fc53781091db95e69d4c3

manifest

@@ -1,5 +1,5 @@
-C Initial\sattempt\sat\sdefining\sthe\ssqlite3_initialize()\sand\ssqlite3_shutdown()\ninterfaces.\s(CVS\s5198)
+C Invalidate\ssqlite3_blob*\shandles\swhenever\san\sSQL\sstatement\sis\sused\sto\sdelete\sor\smodify\sthe\srows\scontaining\sthe\sopen\sblob.\sPreviously,\smodifying\sthe\stable\scontaining\sthe\sopen\sblob\sin\sany\sway\sinvalidated\sthe\shandle.\sThis\swas\stoo\srestrictive.\s(CVS\s5199)
-D 2008-06-09T21:57:23
+D 2008-06-10T17:30:26
 F Makefile.arm-wince-mingw32ce-gcc ac5f7b2cef0cd850d6f755ba6ee4ab961b1fadf7
 F Makefile.in ce92ea8dc7adfb743757794f51c10d1b0d9c55e4
 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654
@@ -96,7 +96,7 @@ F src/attach.c 496cc628b2e8c4d8db99d7c136761fcbebd8420b
 F src/auth.c c8b2ab5c8bad4bd90ed7c294694f48269162c627
 F src/bitvec.c ab50c4b8c6a899dae499f5a805eebe4223c78269
 F src/btmutex.c 483ced3c52205b04b97df69161fadbf87f4f1ea2
-F src/btree.c 0c2c19a9796b5d2cd9f50f6c67559bc923349941
+F src/btree.c 5f76517e78b66d180abb12df2b519f0753745a29
 F src/btree.h b1bd7e0b8c2e33658aaf447cb0d1d94f74664b6b
 F src/btreeInt.h dc04ee33d8eb84714b2acdf81336fbbf6e764530
 F src/build.c a52d9d51341444a2131e3431608f245db80d9591
@@ -338,7 +338,7 @@ F test/in.test 763a29007a4850d611ac4441bfa488fb9969ad30
 F test/in2.test b1f447f4f0f67e9f83ff931e7e2e30873f9ea055
 F test/in3.test dc62b080ed79898121c61c91118b4d1e111f1438
 F test/incrblob.test 4455fffd08b2f9418a9257e18b135d72273eff3e
-F test/incrblob2.test f5b70f9531f8f879ef49516b5205395b2d5ac3c9
+F test/incrblob2.test bb295ab403e4d3a054a31b250a375a32050deb45
 F test/incrblob_err.test 00a8bcb25cb493d53f4efed0f5cf09c386534940
 F test/incrvacuum.test 1a2b0bddc76629afeb41e3d8ea3e4563982d16b9
 F test/incrvacuum2.test 46ef65f377e3937cfd1ba66e818309dab46f590d
@@ -592,7 +592,7 @@ F tool/speedtest16.c c8a9c793df96db7e4933f0852abb7a03d48f2e81
 F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff
 F tool/speedtest8.c 1dbced29de5f59ba2ebf877edcadf171540374d1
 F tool/speedtest8inst1.c c65494ca99d1e09c246dfe37a7ca7a354af9990f
-P 120bffff747592f1ab6ed02713a712cc74c12528
+P 220bfd1f5cef0dfa8b800faa814ad4dc1456ced4
-R 8d8e29a55f02f2444346b9de32d470a6
+R 7091f0d54d253e94a68681a5dbc7d30d
-U drh
+U danielk1977
-Z 41d7f329a14fa126f2a7b2f5f5beb340
+Z 0473705fb37e9902593a0d0168c10fa7

manifest.uuid

@@ -1 +1 @@
-220bfd1f5cef0dfa8b800faa814ad4dc1456ced4
+e339c91f8718482ce74fc53781091db95e69d4c3

src/btree.c

@@ -9,7 +9,7 @@
 **    May you share freely, never taking more than you give.
 **
 *************************************************************************
-** $Id: btree.c,v 1.460 2008/06/09 19:27:12 shane Exp $
+** $Id: btree.c,v 1.461 2008/06/10 17:30:26 danielk1977 Exp $
 **
 ** This file implements a external (disk-based) database using BTrees.
 ** See the header comment on "btreeInt.h" for additional information.
@@ -68,7 +68,7 @@ int sqlite3_enable_shared_cache(int enable){
 /*
 ** Forward declaration
 */
-static int checkReadLocks(Btree*,Pgno,BtCursor*);
+static int checkReadLocks(Btree*, Pgno, BtCursor*, i64);
 
 
 #ifdef SQLITE_OMIT_SHARED_CACHE
@@ -373,11 +373,6 @@ int sqlite3BtreeRestoreOrClearCursorPosition(BtCursor *pCur){
   if( pCur->eState==CURSOR_FAULT ){
     return pCur->skip;
   }
-#ifndef SQLITE_OMIT_INCRBLOB
-  if( pCur->isIncrblobHandle ){
-    return SQLITE_ABORT;
-  }
-#endif
   pCur->eState = CURSOR_INVALID;
   rc = sqlite3BtreeMoveto(pCur, pCur->pKey, 0, pCur->nKey, 0, &pCur->skip);
   if( rc==SQLITE_OK ){
@@ -2717,7 +2712,7 @@ static int btreeCursor(
     if( pBt->readOnly ){
       return SQLITE_READONLY;
     }
-    if( checkReadLocks(p, iTable, 0) ){
+    if( checkReadLocks(p, iTable, 0, 0) ){
       return SQLITE_LOCKED;
     }
   }
@@ -3254,6 +3249,12 @@ int sqlite3BtreeKey(BtCursor *pCur, u32 offset, u32 amt, void *pBuf){
 int sqlite3BtreeData(BtCursor *pCur, u32 offset, u32 amt, void *pBuf){
   int rc;
 
+#ifndef SQLITE_OMIT_INCRBLOB
+  if ( pCur->eState==CURSOR_INVALID ){
+    return SQLITE_ABORT;
+  }
+#endif
+
   assert( cursorHoldsMutex(pCur) );
   rc = restoreOrClearCursorPosition(pCur);
   if( rc==SQLITE_OK ){
@@ -5594,31 +5595,62 @@ static int balance(MemPage *pPage, int insert){
 ** is not in the ReadUncommmitted state, then this routine returns 
 ** SQLITE_LOCKED.
 **
-** In addition to checking for read-locks (where a read-lock 
+** As well as cursors with wrFlag==0, cursors with wrFlag==1 and 
-** means a cursor opened with wrFlag==0) this routine also moves
+** isIncrblobHandle==1 are also considered 'read' cursors. Incremental 
-** all write cursors so that they are pointing to the 
+** blob cursors are used for both reading and writing.
-** first Cell on the root page.  This is necessary because an insert 
+**
-** or delete might change the number of cells on a page or delete
+** When pgnoRoot is the root page of an intkey table, this function is also
-** a page entirely and we do not want to leave any cursors 
+** responsible for invalidating incremental blob cursors when the table row
-** pointing to non-existant pages or cells.
+** on which they are opened is deleted or modified. Cursors are invalidated
+** according to the following rules:
+**
+**   1) When BtreeClearTable() is called to completely delete the contents
+**      of a B-Tree table, pExclude is set to zero and parameter iRow is 
+**      set to non-zero. In this case all incremental blob cursors open
+**      on the table rooted at pgnoRoot are invalidated.
+**
+**   2) When BtreeInsert(), BtreeDelete() or BtreePutData() is called to 
+**      modify a table row via an SQL statement, pExclude is set to the 
+**      write cursor used to do the modification and parameter iRow is set
+**      to the integer row id of the B-Tree entry being modified. Unless
+**      pExclude is itself an incremental blob cursor, then all incremental
+**      blob cursors open on row iRow of the B-Tree are invalidated.
+**
+**   3) If both pExclude and iRow are set to zero, no incremental blob 
+**      cursors are invalidated.
 */
-static int checkReadLocks(Btree *pBtree, Pgno pgnoRoot, BtCursor *pExclude){
+static int checkReadLocks(
+  Btree *pBtree, 
+  Pgno pgnoRoot, 
+  BtCursor *pExclude,
+  i64 iRow
+){
   BtCursor *p;
   BtShared *pBt = pBtree->pBt;
   sqlite3 *db = pBtree->db;
   assert( sqlite3BtreeHoldsMutex(pBtree) );
   for(p=pBt->pCursor; p; p=p->pNext){
     if( p==pExclude ) continue;
-    if( p->eState!=CURSOR_VALID ) continue;
     if( p->pgnoRoot!=pgnoRoot ) continue;
-    if( p->wrFlag==0 ){
+#ifndef SQLITE_OMIT_INCRBLOB
+    if( p->isIncrblobHandle && ( 
+         (!pExclude && iRow)
+      || (pExclude && !pExclude->isIncrblobHandle && p->info.nKey==iRow)
+    )){
+      p->eState = CURSOR_INVALID;
+    }
+#endif
+    if( p->eState!=CURSOR_VALID ) continue;
+    if( p->wrFlag==0 
+#ifndef SQLITE_OMIT_INCRBLOB
+     || p->isIncrblobHandle
+#endif
+    ){
       sqlite3 *dbOther = p->pBtree->db;
       if( dbOther==0 ||
          (dbOther!=db && (dbOther->flags & SQLITE_ReadUncommitted)==0) ){
         return SQLITE_LOCKED;
       }
-    }else if( p->pPage->pgno!=p->pgnoRoot ){
-      moveToRoot(p);
     }
   }
   return SQLITE_OK;
@@ -5669,7 +5701,7 @@ int sqlite3BtreeInsert(
   if( !pCur->wrFlag ){
     return SQLITE_PERM;   /* Cursor not open for writing */
   }
-  if( checkReadLocks(pCur->pBtree, pCur->pgnoRoot, pCur) ){
+  if( checkReadLocks(pCur->pBtree, pCur->pgnoRoot, pCur, nKey) ){
     return SQLITE_LOCKED; /* The table pCur points to has a read lock */
   }
   if( pCur->eState==CURSOR_FAULT ){
@@ -5763,7 +5795,7 @@ int sqlite3BtreeDelete(BtCursor *pCur){
   if( !pCur->wrFlag ){
     return SQLITE_PERM;   /* Did not open this cursor for writing */
   }
-  if( checkReadLocks(pCur->pBtree, pCur->pgnoRoot, pCur) ){
+  if( checkReadLocks(pCur->pBtree, pCur->pgnoRoot, pCur, pCur->info.nKey) ){
     return SQLITE_LOCKED; /* The table pCur points to has a read lock */
   }
 
@@ -6062,7 +6094,7 @@ int sqlite3BtreeClearTable(Btree *p, int iTable){
   pBt->db = p->db;
   if( p->inTrans!=TRANS_WRITE ){
     rc = pBt->readOnly ? SQLITE_READONLY : SQLITE_ERROR;
-  }else if( (rc = checkReadLocks(p, iTable, 0))!=SQLITE_OK ){
+  }else if( (rc = checkReadLocks(p, iTable, 0, 1))!=SQLITE_OK ){
     /* nothing to do */
   }else if( SQLITE_OK!=(rc = saveAllCursors(pBt, iTable, 0)) ){
     /* nothing to do */
@@ -7112,12 +7144,11 @@ int sqlite3BtreePutData(BtCursor *pCsr, u32 offset, u32 amt, void *z){
   assert( cursorHoldsMutex(pCsr) );
   assert( sqlite3_mutex_held(pCsr->pBtree->db->mutex) );
   assert(pCsr->isIncrblobHandle);
-  if( pCsr->eState>=CURSOR_REQUIRESEEK ){
+
-    if( pCsr->eState==CURSOR_FAULT ){
+  restoreOrClearCursorPosition(pCsr);
-      return pCsr->skip;
+  assert( pCsr->eState!=CURSOR_REQUIRESEEK );
-    }else{
+  if( pCsr->eState!=CURSOR_VALID ){
-      return SQLITE_ABORT;
+    return SQLITE_ABORT;
-    }
   }
 
   /* Check some preconditions: 
@@ -7130,7 +7161,7 @@ int sqlite3BtreePutData(BtCursor *pCsr, u32 offset, u32 amt, void *z){
   }
   assert( !pCsr->pBt->readOnly 
           && pCsr->pBt->inTransaction==TRANS_WRITE );
-  if( checkReadLocks(pCsr->pBtree, pCsr->pgnoRoot, pCsr) ){
+  if( checkReadLocks(pCsr->pBtree, pCsr->pgnoRoot, pCsr, 0) ){
     return SQLITE_LOCKED; /* The table pCur points to has a read lock */
   }
   if( pCsr->eState==CURSOR_INVALID || !pCsr->pPage->intKey ){

test/incrblob2.test

@@ -12,7 +12,7 @@
 # Test that it is possible to have two open blob handles on a single
 # blob object.
 #
-# $Id: incrblob2.test,v 1.1 2008/06/09 15:51:27 danielk1977 Exp $
+# $Id: incrblob2.test,v 1.2 2008/06/10 17:30:26 danielk1977 Exp $
 #
 
 set testdir [file dirname $argv0]
@@ -26,9 +26,10 @@ ifcapable {!autovacuum || !pragma || !incrblob} {
 do_test incrblob2-1.0 {
   execsql {
     CREATE TABLE blobs(id INTEGER PRIMARY KEY, data BLOB);
-    INSERT INTO blobs VALUES(0, zeroblob(10240));
+    INSERT INTO blobs VALUES(NULL, zeroblob(5000));
-    INSERT INTO blobs VALUES(1, zeroblob(10240));
+    INSERT INTO blobs VALUES(NULL, zeroblob(5000));
-    INSERT INTO blobs VALUES(2, zeroblob(10240));
+    INSERT INTO blobs VALUES(NULL, zeroblob(5000));
+    INSERT INTO blobs VALUES(NULL, zeroblob(5000));
   }
 } {}
 
@@ -89,11 +90,11 @@ foreach iOffset [list 0 256 4094] {
 
   do_test incrblob2-2.$iOffset.1 {
     set fd1 [db incrblob blobs data 1]
-    seek $fd1 [expr $iOffset - 10240] end
+    seek $fd1 [expr $iOffset - 5000] end
     fconfigure $fd1 -buffering none
 
     set fd2 [db incrblob blobs data 1]
-    seek $fd2 [expr $iOffset - 10240] end
+    seek $fd2 [expr $iOffset - 5000] end
     fconfigure $fd2 -buffering none
 
     puts -nonewline $fd1 "123456"
@@ -115,17 +116,176 @@ do_test incrblob2-3.1 {
 } {}
 do_test incrblob2-3.2 {
   execsql {
-    INSERT INTO blobs VALUES(4, zeroblob(10240));
+    INSERT INTO blobs VALUES(5, zeroblob(10240));
   }
 } {}
 do_test incrblob2-3.3 {
   set rc [catch { read $fd1 6 } msg]
   list $rc $msg
-} "1 {error reading \"$fd1\": interrupted system call}"
+} {0 123456}
 do_test incrblob2-3.4 {
   close $fd1
 } {}
 
+#--------------------------------------------------------------------------
+# The following tests - incrblob2-4.* - test that blob handles are 
+# invalidated at the correct times.
+#
+do_test incrblob2-4.1 {
+  db eval BEGIN
+  db eval { CREATE TABLE t1(id INTEGER PRIMARY KEY, data BLOB); }
+  for {set ii 1} {$ii < 100} {incr ii} {
+    set data [string repeat "blob$ii" 500]
+    db eval { INSERT INTO t1 VALUES($ii, $data) }
+  }
+  db eval COMMIT
+} {}
+
+proc aborted_handles {} {
+  global handles
+
+  set aborted {}
+  for {set ii 1} {$ii < 100} {incr ii} {
+    set str "blob$ii"
+    set nByte [string length $str]
+    set iOffset [expr $nByte * $ii * 2]
+
+    set rc [catch {sqlite3_blob_read $handles($ii) $iOffset $nByte} msg]
+    if {$rc && $msg eq "SQLITE_ABORT"} {
+      lappend aborted $ii
+    } else {
+      if {$rc || $msg ne $str} {
+        error "blob $ii: $msg"
+      }
+    }
+  }
+  set aborted
+}
+
+do_test incrblob2-4.2 {
+  for {set ii 1} {$ii < 100} {incr ii} {
+    set handles($ii) [db incrblob t1 data $ii]
+  }
+  aborted_handles
+} {}
+
+# Update row 3. This should abort handle 3 but leave all others untouched.
+#
+do_test incrblob2-4.3 {
+  db eval {UPDATE t1 SET data = data || '' WHERE id = 3}
+  aborted_handles
+} {3}
+
+# Test that a write to handle 3 also returns SQLITE_ABORT.
+#
+do_test incrblob2-4.3.1 {
+  set rc [catch {sqlite3_blob_write $::handles(3) 10 HELLO} msg]
+  list $rc $msg
+} {1 SQLITE_ABORT}
+
+# Delete row 14. This should abort handle 6 but leave all others untouched.
+#
+do_test incrblob2-4.4 {
+  db eval {DELETE FROM t1 WHERE id = 14}
+  aborted_handles
+} {3 14}
+
+# Change the rowid of row 15 to 102. Should abort handle 15.
+#
+do_test incrblob2-4.5 {
+  db eval {UPDATE t1 SET id = 102 WHERE id = 15}
+  aborted_handles
+} {3 14 15}
+
+# Clobber row 92 using INSERT OR REPLACE.
+#
+do_test incrblob2-4.6 {
+  db eval {INSERT OR REPLACE INTO t1 VALUES(92, zeroblob(1000))}
+  aborted_handles
+} {3 14 15 92}
+
+# Clobber row 65 using UPDATE OR REPLACE on row 35. This should abort 
+# handles 35 and 65.
+#
+do_test incrblob2-4.7 {
+  db eval {UPDATE OR REPLACE t1 SET id = 65 WHERE id = 35}
+  aborted_handles
+} {3 14 15 35 65 92}
+
+# Insert a couple of new rows. This should not invalidate any handles.
+#
+do_test incrblob2-4.9 {
+  db eval {INSERT INTO t1 SELECT NULL, data FROM t1}
+  aborted_handles
+} {3 14 15 35 65 92}
+
+# Delete all rows from 1 to 25. This should abort all handles up to 25.
+#
+do_test incrblob2-4.9 {
+  db eval {DELETE FROM t1 WHERE id >=1 AND id <= 25}
+  aborted_handles
+} {1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 35 65 92}
+
+# Delete the whole table (this will use sqlite3BtreeClearTable()). All handles
+# should now be aborted.
+#
+do_test incrblob2-4.10 {
+  db eval {DELETE FROM t1}
+  aborted_handles
+} {1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99}
+
+do_test incrblob2-4.1.X {
+  for {set ii 1} {$ii < 100} {incr ii} {
+    close $handles($ii) 
+  }
+} {}
+
+#--------------------------------------------------------------------------
+# The following tests - incrblob2-5.* - test that in shared cache an open
+# blob handle counts as a read-lock on its table.
+#
+ifcapable shared_cache {
+  db close
+  set ::enable_shared_cache [sqlite3_enable_shared_cache 1]
+
+  do_test incrblob2-5.1 {
+    sqlite3 db test.db
+    sqlite3 db2 test.db
+
+    execsql {
+      INSERT INTO t1 VALUES(1, 'abcde');
+    }
+  } {}
+
+  do_test incrblob2-5.2 {
+    catchsql { INSERT INTO t1 VALUES(2, 'fghij') } db2
+  } {0 {}}
+
+  do_test incrblob2-5.3 {
+    set blob [db incrblob t1 data 1]
+    catchsql { INSERT INTO t1 VALUES(3, 'klmno') } db2
+  } {1 {database is locked}}
+
+  do_test incrblob2-5.4 {
+    close $blob
+    execsql BEGIN db2
+    catchsql { INSERT INTO t1 VALUES(4, 'pqrst') } db2
+  } {0 {}}
+
+  do_test incrblob2-5.5 {
+    set blob [db incrblob -readonly t1 data 1]
+    catchsql { INSERT INTO t1 VALUES(5, 'uvwxy') } db2
+  } {1 {database table is locked}}
+
+  do_test incrblob2-5.6 {
+    close $blob
+    catchsql { INSERT INTO t1 VALUES(3, 'klmno') } db2
+  } {0 {}}
+
+  db2 close
+  db close
+  sqlite3_enable_shared_cache $::enable_shared_cache
+}
 
 finish_test