]> git.ipfire.org Git - thirdparty/sqlite.git/commitdiff
Only allow UNLOCKED transactions to commit if none of the pages read by the transacti...
authordan <dan@noemail.net>
Wed, 29 Jul 2015 12:14:28 +0000 (12:14 +0000)
committerdan <dan@noemail.net>
Wed, 29 Jul 2015 12:14:28 +0000 (12:14 +0000)
FossilOrigin-Name: 0b9718426e44df092850c5d095ce1b84a1e567cf

manifest
manifest.uuid
src/btree.c
src/btree.h
src/pager.c
src/pager.h
src/vdbeaux.c
src/wal.c
src/wal.h
test/unlocked.test

index c4c044ae5ccc563fc93009b7eefc31d5abee1555..2f36ddd5b8b6063cd794feffae24b08b438cb7a8 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Add\ssome\stest\scases\sand\sfix\ssome\ssmall\sproblems\swith\sBEGIN\sUNLOCKED\stransactions.
-D 2015-07-28T16:46:49.291
+C Only\sallow\sUNLOCKED\stransactions\sto\scommit\sif\snone\sof\sthe\spages\sread\sby\sthe\stransaction\shave\sbeen\smodified\ssince\sit\swas\sopened.
+D 2015-07-29T12:14:28.276
 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f
 F Makefile.in 4de3ef40c8b3b75c0c55ff4242a43c8ce1ad90ee
 F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23
@@ -270,8 +270,8 @@ F src/auth.c b56c78ebe40a2110fd361379f7e8162d23f92240
 F src/backup.c 4d9134dc988a87838c06056c89c0e8c4700a0452
 F src/bitvec.c d1f21d7d91690747881f03940584f4cc548c9d3d
 F src/btmutex.c 45a968cc85afed9b5e6cf55bf1f42f8d18107f79
-F src/btree.c b184ce6d483dfadc53126a4b7994633ec0d5ef5f
-F src/btree.h 550e8447684013a69196ec664142f67538da8ef1
+F src/btree.c 5087b0a1358abd1449165dd14d49962bc3ad9f44
+F src/btree.h 40bd41ef0b71d6f7502725dc159fa0d6bebd8bb3
 F src/btreeInt.h 2ad754dd4528baa8d0946a593cc373b890bf859e
 F src/build.c 28c15c43eefc0066ff64040526ff649c32fe5523
 F src/callback.c 7b44ce59674338ad48b0e84e7b72f935ea4f68b0
@@ -315,8 +315,8 @@ F src/os_setup.h c9d4553b5aaa6f73391448b265b89bed0b890faa
 F src/os_unix.c 388c023582b17890f10c980b30ec1922b471753b
 F src/os_win.c 40b3af7a47eb1107d0d69e592bec345a3b7b798a
 F src/os_win.h eb7a47aa17b26b77eb97e4823f20a00b8bda12ca
-F src/pager.c c9f22eb831851220c1095863c5797a88f71bea8a
-F src/pager.h b6cf6e9552c5b1e7d6d0d8cd7ca51be7802bec72
+F src/pager.c 18aa67541bd2c7d7963f180669ea4e48cd5246f4
+F src/pager.h 41938212a5d80a7068883bb29ea6ac0d4ef3a6d8
 F src/parse.y 199145cd982587d70fa3db2f9b11f7ab118f0acd
 F src/pcache.c cde06aa50962595e412d497e22fd2e07878ba1f0
 F src/pcache.h 9968603796240cdf83da7e7bef76edf90619cea9
@@ -395,15 +395,15 @@ F src/vdbe.c fd6832408d9621ad347a65b01a05380fe766232f
 F src/vdbe.h 7a75045d879118b9d3af7e8b3c108f2f27c51473
 F src/vdbeInt.h 8b54e01ad0463590e7cffabce0bc36da9ee4f816
 F src/vdbeapi.c adabbd66eb2e3a10f3998485ee0be7e326d06ee4
-F src/vdbeaux.c 27feae106f3f4db298e65d37a9da4962aaff724b
+F src/vdbeaux.c 24a03ccbadc8d4cfecf25b6399db95851b10dd21
 F src/vdbeblob.c 4f2e8e075d238392df98c5e03a64342465b03f90
 F src/vdbemem.c ae38a0d35ae71cf604381a887c170466ba518090
 F src/vdbesort.c f5009e7a35e3065635d8918b9a31f498a499976b
 F src/vdbetrace.c 8befe829faff6d9e6f6e4dee5a7d3f85cc85f1a0
 F src/vtab.c 082b35a25a26e3d36f365ca8cd73c1922532f05e
 F src/vxworks.h c18586c8edc1bddbc15c004fa16aeb1e1342b4fb
-F src/wal.c a2e35f042103519ce22050acaf167960446963c7
-F src/wal.h 5188cd85398dc612ae5ed2ed02d193377218be56
+F src/wal.c 4497b466fee8ed56c31672bb35741dd0321ec999
+F src/wal.h cd7ea52439d56571d549b7d4eacdeed58d841385
 F src/walker.c c253b95b4ee44b21c406e2a1052636c31ea27804
 F src/where.c 909eba3b6db984eb2adfbca9de2c237ee7056adb
 F src/whereInt.h 5f87e3c4b0551747d119730dfebddd3c54f04047
@@ -1215,7 +1215,7 @@ F test/types3.test 99e009491a54f4dc02c06bdbc0c5eea56ae3e25a
 F test/unique.test 93f8b2ef5ea51b9495f8d6493429b1fd0f465264
 F test/unique2.test 41e7f83c6827605991160a31380148a9fc5f1339
 F test/unixexcl.test cd6c765f75e50e8e2c2ba763149e5d340ea19825
-F test/unlocked.test 854f3f428bb2e0ae91cb2400596a8f1ab7eaa409
+F test/unlocked.test 2da6645c3532985715d17dfcb240ef5f09c2e9d4
 F test/unordered.test ca7adce0419e4ca0c50f039885e76ed2c531eda8
 F test/update.test 6c68446b8a0a33d522a7c72b320934596a2d7d32
 F test/uri.test 23662b7b61958b0f0e47082de7d06341ccf85d5b
@@ -1367,7 +1367,7 @@ F tool/vdbe_profile.tcl 67746953071a9f8f2f668b73fe899074e2c6d8c1
 F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4
 F tool/warnings.sh 48bd54594752d5be3337f12c72f28d2080cb630b
 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f
-P 8079421604dbd40d03471dad6d12115119b554c2
-R 52e6230fe0fe2d8cb4434e54d864e964
+P 6da0e962ad2aa5e52c1f1b5c3dbf77a2cb16ac2d
+R 74dcd93afc2d7e4cc222b666276012ce
 U dan
-Z 849da1cda4069d7075ecc7190a9058c5
+Z 3bd2cb4e2cf9d30cb919b0a0d11717fa
index 23abb2a1991db65d5baf635b7f56c461a51e2a12..f6215ad409177012defd3a757286540ddcd504cd 100644 (file)
@@ -1 +1 @@
-6da0e962ad2aa5e52c1f1b5c3dbf77a2cb16ac2d
\ No newline at end of file
+0b9718426e44df092850c5d095ce1b84a1e567cf
\ No newline at end of file
index c83a01ad4cb2bf41f4fb0e18fc0d5ed4e9bfd4a7..69d4873d0de2aef84a6fa8852ec33245cf12db91 100644 (file)
@@ -9590,15 +9590,3 @@ int sqlite3BtreeIsReadonly(Btree *p){
 ** Return the size of the header added to each page by this module.
 */
 int sqlite3HeaderSizeBtree(void){ return ROUND8(sizeof(MemPage)); }
-
-int sqlite3BtreeExclusiveLock(Btree *p){
-  int rc;
-  BtShared *pBt = p->pBt;
-  sqlite3BtreeEnter(p);
-  rc = sqlite3PagerExclusiveLock(pBt->pPager, pBt->pPage1->pDbPage);
-  sqlite3BtreeLeave(p);
-  return rc;
-}
-
-
-
index ae9b2c4e59951000be0d0e0cd42b1205a92b9863..50dc8ca2906c580da4acb94194c579dc23107fea 100644 (file)
@@ -270,6 +270,4 @@ void sqlite3BtreeCursorList(Btree*);
 # define sqlite3SchemaMutexHeld(X,Y,Z) 1
 #endif
 
-int sqlite3BtreeExclusiveLock(Btree*);
-
 #endif /* _BTREE_H_ */
index 2ea3b90a96b0f9c84b50e50384cf69302cedd1d9..5107edda361da155428c2f7330a12b252012f13e 100644 (file)
@@ -657,6 +657,7 @@ struct Pager {
   u32 cksumInit;              /* Quasi-random value added to every checksum */
   u32 nSubRec;                /* Number of records written to sub-journal */
   Bitvec *pInJournal;         /* One bit for each page in the database file */
+  Bitvec *pAllRead;           /* Pages read within current UNLOCKED trans. */
   sqlite3_file *fd;           /* File descriptor for database */
   sqlite3_file *jfd;          /* File descriptor for main journal */
   sqlite3_file *sjfd;         /* File descriptor for sub-journal */
@@ -1738,6 +1739,16 @@ static int addToSavepointBitvecs(Pager *pPager, Pgno pgno){
   return rc;
 }
 
+/*
+** Free the Pager.pInJournal and Pager.pAllRead bitvec objects.
+*/
+static void pagerFreeBitvecs(Pager *pPager){
+  sqlite3BitvecDestroy(pPager->pInJournal);
+  sqlite3BitvecDestroy(pPager->pAllRead);
+  pPager->pInJournal = 0;
+  pPager->pAllRead = 0;
+}
+
 /*
 ** This function is a no-op if the pager is in exclusive mode and not
 ** in the ERROR state. Otherwise, it switches the pager to PAGER_OPEN
@@ -1762,8 +1773,7 @@ static void pager_unlock(Pager *pPager){
        || pPager->eState==PAGER_ERROR 
   );
 
-  sqlite3BitvecDestroy(pPager->pInJournal);
-  pPager->pInJournal = 0;
+  pagerFreeBitvecs(pPager);
   releaseAllSavepoints(pPager);
 
   if( pagerUseWal(pPager) ){
@@ -1999,8 +2009,7 @@ static int pager_end_transaction(Pager *pPager, int hasMaster, int bCommit){
   }
 #endif
 
-  sqlite3BitvecDestroy(pPager->pInJournal);
-  pPager->pInJournal = 0;
+  pagerFreeBitvecs(pPager);
   pPager->nRec = 0;
   sqlite3PcacheCleanAll(pPager->pPCache);
   sqlite3PcacheTruncate(pPager->pPCache, pPager->dbSize);
@@ -4082,7 +4091,7 @@ static int syncJournal(Pager *pPager, int newHdr){
   assert( assert_pager_state(pPager) );
   assert( !pagerUseWal(pPager) );
 
-  rc = sqlite3PagerExclusiveLock(pPager, 0);
+  rc = sqlite3PagerExclusiveLock(pPager);
   if( rc!=SQLITE_OK ) return rc;
 
   if( !pPager->noSync ){
@@ -4432,7 +4441,7 @@ static int pagerStress(void *p, PgHdr *pPg){
   if( pagerUseWal(pPager) ){
     /* If the transaction is a "BEGIN UNLOCKED" transaction, the page 
     ** cannot be flushed to disk. Return early in this case. */
-    if( sqlite3WalIsInTrans(pPager->pWal)==0 ) return SQLITE_OK;
+    if( pPager->pAllRead ) return SQLITE_OK;
 
     /* Write a single frame for this page to the log. */
     rc = subjournalPageIfRequired(pPg); 
@@ -5274,6 +5283,14 @@ int sqlite3PagerAcquire(
   }
   pPager->hasBeenUsed = 1;
 
+  /* If this is an UNLOCKED transaction and the page being read was
+  ** present in the database file when the transaction was opened,
+  ** mark it as read in the pAllRead vector.  */
+  if( pPager->pAllRead && pgno<=pPager->dbOrigSize ){
+    rc = sqlite3BitvecSet(pPager->pAllRead, pgno);
+    if( rc!=SQLITE_OK ) goto pager_acquire_err;
+  }
+
   /* If the pager is in the error state, return an error immediately. 
   ** Otherwise, request the page from the PCache layer. */
   if( pPager->errCode!=SQLITE_OK ){
@@ -5578,6 +5595,7 @@ int sqlite3PagerBegin(Pager *pPager, int exFlag, int subjInMemory){
 
   if( ALWAYS(pPager->eState==PAGER_READER) ){
     assert( pPager->pInJournal==0 );
+    assert( pPager->pAllRead==0 );
 
     if( pagerUseWal(pPager) ){
       /* If the pager is configured to use locking_mode=exclusive, and an
@@ -5598,6 +5616,13 @@ int sqlite3PagerBegin(Pager *pPager, int exFlag, int subjInMemory){
       */
       if( exFlag>=0 ){
         rc = sqlite3WalBeginWriteTransaction(pPager->pWal);
+      }else{
+        pPager->pAllRead = sqlite3BitvecCreate(pPager->dbSize);
+        if( pPager->pAllRead==0 ){
+          rc = SQLITE_NOMEM;
+        }else{
+          rc = sqlite3BitvecSet(pPager->pAllRead, 1);
+        }
       }
     }else{
       /* Obtain a RESERVED lock on the database file. If the exFlag parameter
@@ -6066,7 +6091,7 @@ int sqlite3PagerSync(Pager *pPager, const char *zMaster){
 ** Otherwise, either SQLITE_BUSY or an SQLITE_IOERR_XXX error code is 
 ** returned.
 */
-int sqlite3PagerExclusiveLock(Pager *pPager, PgHdr *pPage1){
+int sqlite3PagerExclusiveLock(Pager *pPager){
   int rc = SQLITE_OK;
   assert( pPager->eState==PAGER_WRITER_CACHEMOD 
        || pPager->eState==PAGER_WRITER_DBMOD 
@@ -6076,21 +6101,14 @@ int sqlite3PagerExclusiveLock(Pager *pPager, PgHdr *pPage1){
   if( 0==pagerUseWal(pPager) ){
     rc = pager_wait_on_lock(pPager, EXCLUSIVE_LOCK);
   }else{
-    Wal *pWal = pPager->pWal;
-    if( 0==sqlite3WalIsInTrans(pWal) ){
-      /* TODO: There must be an optimization opportunity here, as this call 
-      ** to PcacheDirtyList() sorts the list of dirty pages, even though it 
-      ** is not really required - and will be sorted again in CommitPhaseOne()
-      ** in any case.  */
-      PgHdr *pList = sqlite3PcacheDirtyList(pPager->pPCache);
-
+    if( pPager->pAllRead ){
       /* This is an UNLOCKED transaction. Attempt to lock the wal database
       ** here. If SQLITE_BUSY (but not SQLITE_BUSY_SNAPSHOT) is returned,
       ** invoke the busy-handler and try again for as long as it returns
       ** non-zero.  */
       do {
         /* rc = sqlite3WalBeginWriteTransaction(pWal); */
-        rc = sqlite3WalLockForCommit(pWal, pList, pPage1); 
+        rc = sqlite3WalLockForCommit(pPager->pWal, pPager->pAllRead);
       }while( rc==SQLITE_BUSY 
            && pPager->xBusyHandler(pPager->pBusyHandlerArg) 
       );
index 57d2812701044ab0692f0286cef1de592534dafa..3c0b9a1a715ff92949cedb49a4c5feafb88863f5 100644 (file)
@@ -150,7 +150,7 @@ void *sqlite3PagerGetExtra(DbPage *);
 void sqlite3PagerPagecount(Pager*, int*);
 int sqlite3PagerBegin(Pager*, int exFlag, int);
 int sqlite3PagerCommitPhaseOne(Pager*,const char *zMaster, int);
-int sqlite3PagerExclusiveLock(Pager*, DbPage *pPage1);
+int sqlite3PagerExclusiveLock(Pager*);
 int sqlite3PagerSync(Pager *pPager, const char *zMaster);
 int sqlite3PagerCommitPhaseTwo(Pager*);
 int sqlite3PagerRollback(Pager*);
index f5f9d0fbdda4feda9a6ba963f0befa9396feb456..93a7187ab97774c6809f89906f299dfd95010ed8 100644 (file)
@@ -2020,7 +2020,9 @@ static int vdbeCommit(sqlite3 *db, Vdbe *p){
     if( sqlite3BtreeIsInTrans(pBt) ){
       needXcommit = 1;
       if( i!=1 ) nTrans++;
-      rc = sqlite3BtreeExclusiveLock(pBt);
+      sqlite3BtreeEnter(pBt);
+      rc = sqlite3PagerExclusiveLock(sqlite3BtreePager(pBt));
+      sqlite3BtreeLeave(pBt);
     }
   }
 
index be018ebc9964d37a47ea4aff36aa7c56597b239c..5fbdae6a1adbb4abba2ed22deeab2aae5e14d683 100644 (file)
--- a/src/wal.c
+++ b/src/wal.c
@@ -2573,7 +2573,7 @@ int sqlite3WalBeginWriteTransaction(Wal *pWal){
 ** transaction. It may be assumed that no frames have been written to
 ** the wal file.
 */
-int sqlite3WalLockForCommit(Wal *pWal, PgHdr *pList, PgHdr *pPage1){
+int sqlite3WalLockForCommit(Wal *pWal, Bitvec *pAllRead){
   int rc = walWriteLock(pWal);
 
   /* If the database has been modified since this transaction was started,
@@ -2590,49 +2590,35 @@ int sqlite3WalLockForCommit(Wal *pWal, PgHdr *pList, PgHdr *pPage1){
     volatile WalIndexHdr *pHead;    /* Head of the wal file */
     pHead = walIndexHdr(pWal);
     if( memcmp(&pWal->hdr, (void*)pHead, sizeof(WalIndexHdr))!=0 ){
-      int bSeenPage1 = 0;           /* True if page 1 is in list pList */
-  
       /* TODO: Is this safe? Because it holds the WRITER lock this thread
-      ** has exclusive access to the live header, but might it be corrupt? */
-      PgHdr *pPg;
-      u32 iLast = pHead->mxFrame;
-      for(pPg=pList; rc==SQLITE_OK && pPg; pPg=pPg->pDirty){
-        u32 iSlot = 0;
-        rc = walFindFrame(pWal, pPg->pgno, iLast, &iSlot);
-        if( iSlot>pWal->hdr.mxFrame ){
-          sqlite3_log(SQLITE_OK,
-              "cannot commit UNLOCKED transaction (conflict at page %d)",
-              (int)pPg->pgno
-          );
-          rc = SQLITE_BUSY_SNAPSHOT;
-        }
-        if( pPg->pgno==1 ) bSeenPage1 = 1;
-      }
-  
-      /* If the current transaction does not modify page 1 of the database,
-      ** check if page 1 has been modified since the transaction was started.
-      ** If it has, check if the schema cookie value (the 4 bytes beginning at
-      ** byte offset 40) is the same as it is on pPage1. If not, this indicates
-      ** that the current head of the wal file uses a different schema than 
-      ** the snapshot against which the current transaction was prepared. Return
-      ** SQLITE_BUSY_SNAPSHOT in this case.  */
-      if( rc==SQLITE_OK && bSeenPage1==0 ){
-        u32 iSlot = 0;
-        rc = walFindFrame(pWal, 1, iLast, &iSlot);
-        if( rc==SQLITE_OK && iSlot>pWal->hdr.mxFrame ){
-          u8 aNew[4];
-          u8 *aOld = &((u8*)pPage1->pData)[40];
-          int sz;
-          i64 iOffset;
-          sz = pWal->hdr.szPage;
-          sz = (sz&0xfe00) + ((sz&0x0001)<<16);
-          iOffset = walFrameOffset(iSlot, sz) + WAL_FRAME_HDRSIZE + 40;
-          rc = sqlite3OsRead(pWal->pWalFd, aNew, sizeof(aNew), iOffset);
-          if( rc==SQLITE_OK && memcmp(aOld, aNew, sizeof(aNew)) ){
-            /* TODO: New error code? SQLITE_BUSY_SCHEMA. */
-            rc = SQLITE_BUSY_SNAPSHOT;
+      ** has exclusive access to the live header, but might it be corrupt? 
+      ** This code should check that the wal-index-header is Ok, and return
+      ** SQLITE_BUSY_SNAPSHOT if it is not. */
+      int iHash;
+      int iLastHash = walFramePage(pHead->mxFrame);
+      for(iHash=walFramePage(pWal->hdr.mxFrame+1); iHash<=iLastHash; iHash++){
+        volatile ht_slot *aHash;
+        volatile u32 *aPgno;
+        u32 iZero;
+
+        rc = walHashGet(pWal, iHash, &aHash, &aPgno, &iZero);
+        if( rc==SQLITE_OK ){
+          int i;
+          int iMin = (pWal->hdr.mxFrame+1 - iZero);
+          int iMax = (iHash==0) ? HASHTABLE_NPAGE_ONE : HASHTABLE_NPAGE;
+          if( iMin<1 ) iMin = 1;
+          if( iMax>pHead->mxFrame ) iMax = pHead->mxFrame;
+          for(i=iMin; i<=iMax; i++){
+            if( sqlite3BitvecTestNotNull(pAllRead, aPgno[i]) ){
+              sqlite3_log(SQLITE_OK,
+                  "cannot commit UNLOCKED transaction (conflict at page %d)",
+                  (int)aPgno[i]
+              );
+              rc = SQLITE_BUSY_SNAPSHOT;
+            }
           }
         }
+        if( rc!=SQLITE_OK ) break;
       }
     }
   }
@@ -3293,13 +3279,6 @@ int sqlite3WalHeapMemory(Wal *pWal){
   return (pWal && pWal->exclusiveMode==WAL_HEAPMEMORY_MODE );
 }
 
-/*
-** Return true if in a write transaction, false otherwise.
-*/
-int sqlite3WalIsInTrans(Wal *pWal){
-  return (int)pWal->writeLock;
-}
-
 #ifdef SQLITE_ENABLE_ZIPVFS
 /*
 ** If the argument is not NULL, it points to a Wal object that holds a
index db341caa044eec1fe2c3c4f9c6e3d0e510a83646..fe85a52cc03e9881324affc7e5e952939387018f 100644 (file)
--- a/src/wal.h
+++ b/src/wal.h
@@ -127,8 +127,7 @@ int sqlite3WalExclusiveMode(Wal *pWal, int op);
 int sqlite3WalHeapMemory(Wal *pWal);
 
 /* Return true if the WRITER lock is held. False otherwise. */
-int sqlite3WalIsInTrans(Wal *pWal);
-int sqlite3WalLockForCommit(Wal *pWal, PgHdr *pDirtyList, PgHdr *pPage1); 
+int sqlite3WalLockForCommit(Wal *pWal, Bitvec *pRead);
 int sqlite3WalCommitRequiresUpgrade(Wal *pWal);
 
 #ifdef SQLITE_ENABLE_ZIPVFS
index c7ae0466e5c431e08723a3eda170664d6cacd9e6..2a5862d3ac5ce3fcf470f4dc8fcee88b7c238d78 100644 (file)
@@ -300,13 +300,12 @@ do_multiclient_test tn {
   catch { sql1 ROLLBACK }
 
   #-----------------------------------------------------------------------
-  # The "schema cookie" issue.
   #
   # 1. Begin an UNLOCKED write to "t1" using [db]
   #
-  # 2. Lots of inserts into t2. Enough to grow the db file.
+  # 2. Lots of inserts into t2. Enough to grow the db file and modify page 1.
   #
-  # 3. Check that the UNLOCKED transaction can still be committed.
+  # 3. Check that the UNLOCKED transaction can not be committed.
   #
   do_test 2.$tn.6.1 {
     sql1 {
@@ -324,12 +323,17 @@ do_multiclient_test tn {
   } {}
 
   do_test 2.$tn.6.3 {
+    sql1 { SELECT count(*) FROM t2 }
+    list [catch { sql1 { COMMIT } } msg] $msg [sqlite3_errcode db]
+  } {1 {database is locked} SQLITE_BUSY_SNAPSHOT}
+  sql1 ROLLBACK
+
+  do_test 2.$tn.6.4 {
     sql1 {
-      SELECT count(*) FROM t2;
-      COMMIT;
+      SELECT count(*) FROM t1;
       SELECT count(*) FROM t2;
     }
-  } {1 10001}
+  } {5 10001}
 
   #-----------------------------------------------------------------------
   #