From: dan Date: Thu, 27 Aug 2015 17:42:38 +0000 (+0000) Subject: Fix a problem whereby concurrent transactions would not consider pages read by the... X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=987f821f791502cc5ccb83caa8bf399427c4d94d;p=thirdparty%2Fsqlite.git Fix a problem whereby concurrent transactions would not consider pages read by the transaction before the first write statement. FossilOrigin-Name: fc17f73170a27c2fe511ed6b6d488535c4e35bae --- diff --git a/manifest b/manifest index 4792a3a28f..4440e87c40 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\san\sassert()\sin\spager.c\sthat\scould\sfail\sin\sa\sconcurrent\stransaction. -D 2015-08-26T18:54:45.787 +C Fix\sa\sproblem\swhereby\sconcurrent\stransactions\swould\snot\sconsider\spages\sread\sby\sthe\stransaction\sbefore\sthe\sfirst\swrite\sstatement. +D 2015-08-27T17:42:38.260 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in e2218eb228374422969de7b1680eda6864affcef F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -279,7 +279,7 @@ F src/auth.c b56c78ebe40a2110fd361379f7e8162d23f92240 F src/backup.c 4d9134dc988a87838c06056c89c0e8c4700a0452 F src/bitvec.c d1f21d7d91690747881f03940584f4cc548c9d3d F src/btmutex.c 45a968cc85afed9b5e6cf55bf1f42f8d18107f79 -F src/btree.c aacef0cd0c57c2a1b2ed8a27794fc9e20b6e7a90 +F src/btree.c 88ff0f5bdcd200e4b8cdc2f49a89369112a592d7 F src/btree.h 00d4cdb747c4172a5566faf037116985dbbc377e F src/btreeInt.h 171864bcd81635583dab7b8a04b19b454b18ef80 F src/build.c 1b5814e0eeaba096ae3ee17ebd139d2a25af7250 @@ -324,8 +324,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 bb8e237a54d162a6cc22503f71895851dae9a0bb -F src/pager.h 1335b624cd540815c8c977172589d208d1c251a6 +F src/pager.c 732f3b107ac7180e4c628114a5d06dda00393080 +F src/pager.h f00930ca3bfc0f3298b08a69ed7b920e89b531de F src/parse.y 1e645cacb93979c59f2a510ee2c100e769bd5e3c F src/pcache.c cde06aa50962595e412d497e22fd2e07878ba1f0 F src/pcache.h 9968603796240cdf83da7e7bef76edf90619cea9 @@ -525,7 +525,7 @@ F test/colmeta.test 2c765ea61ee37bc43bbe6d6047f89004e6508eb1 F test/colname.test 08948a4809d22817e0e5de89c7c0a8bd90cb551b F test/concfault.test 500f17c3fcfe7705114422bcc6ddd3c740001a43 F test/concurrent.test 634b6a88f1942f5d68cc89d4d5efa2b11ba7913c -F test/concurrent2.test de43cd6703360dc6268907f1617f0d353d8a43c1 +F test/concurrent2.test d42aaa1d0aaf2c41c8d5de204962b125b411557d F test/concurrent3.test 0a5f7e3036d1eccf0782d7153ac21f5f222e9468 F test/conflict.test 841bcf7cabbfca39c577eb8411ea8601843b46a8 F test/conflict2.test 0d3af4fb534fa1bd020c79960bb56e4d52655f09 @@ -1382,7 +1382,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 a0566382d564ca17fd13475a44fed8f714742d97 -R 82cb8297df85a5e2338dab90f8e6448e +P 69394ddaa2bc9d26477b4359c676c598b733ac9f +R aacab69ab35db4167de1724271774606 U dan -Z 29e210082fd7b7d5f83c5decd49ba489 +Z d9ef073e0b6bc6533e113c00c2daa9fc diff --git a/manifest.uuid b/manifest.uuid index fc6505589c..72a90a6e05 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -69394ddaa2bc9d26477b4359c676c598b733ac9f \ No newline at end of file +fc17f73170a27c2fe511ed6b6d488535c4e35bae \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index 70c36a933c..9986c95325 100644 --- a/src/btree.c +++ b/src/btree.c @@ -595,15 +595,16 @@ static void btreePtrmapEnd(BtShared *pBt, int op, int iSvpt){ */ static int btreePtrmapAllocate(BtShared *pBt){ int rc = SQLITE_OK; - BtreePtrmap *pMap = sqlite3_malloc(sizeof(BtreePtrmap)); - assert( pBt->pMap==0 && sqlite3PagerIsConcurrent(pBt->pPager) ); - if( pMap==0 ){ - rc = SQLITE_NOMEM; - }else{ - memset(&pBt->pPage1->aData[32], 0, sizeof(u32)*2); - memset(pMap, 0, sizeof(BtreePtrmap)); - pMap->iFirst = pBt->nPage + 1; - pBt->pMap = pMap; + if( pBt->pMap==0 ){ + BtreePtrmap *pMap = sqlite3_malloc(sizeof(BtreePtrmap)); + if( pMap==0 ){ + rc = SQLITE_NOMEM; + }else{ + memset(&pBt->pPage1->aData[32], 0, sizeof(u32)*2); + memset(pMap, 0, sizeof(BtreePtrmap)); + pMap->iFirst = pBt->nPage + 1; + pBt->pMap = pMap; + } } return rc; } @@ -3273,6 +3274,7 @@ int sqlite3BtreeBeginTrans(Btree *p, int wrflag){ sqlite3 *pBlock = 0; BtShared *pBt = p->pBt; int rc = SQLITE_OK; + int bConcurrent = (p->db->bConcurrent && !ISAUTOVACUUM); sqlite3BtreeEnter(p); btreeIntegrity(p); @@ -3339,18 +3341,11 @@ int sqlite3BtreeBeginTrans(Btree *p, int wrflag){ if( (pBt->btsFlags & BTS_READ_ONLY)!=0 ){ rc = SQLITE_READONLY; }else{ - int exFlag = (p->db->bConcurrent && !ISAUTOVACUUM) ? -1 : (wrflag>1); - int bSubjInMem = sqlite3TempInMemory(p->db); - assert( p->db->bConcurrent==0 || wrflag==1 ); - rc = sqlite3PagerBegin(pBt->pPager, exFlag, bSubjInMem); + int exFlag = bConcurrent ? -1 : (wrflag>1); + rc = sqlite3PagerBegin(pBt->pPager, exFlag, sqlite3TempInMemory(p->db)); if( rc==SQLITE_OK ){ rc = newDatabase(pBt); } -#ifdef SQLITE_ENABLE_CONCURRENT - if( rc==SQLITE_OK && sqlite3PagerIsConcurrent(pBt->pPager) ){ - rc = btreePtrmapAllocate(pBt); - } -#endif } } @@ -3402,6 +3397,15 @@ int sqlite3BtreeBeginTrans(Btree *p, int wrflag){ trans_begun: +#ifdef SQLITE_ENABLE_CONCURRENT + if( bConcurrent && rc==SQLITE_OK && sqlite3PagerIsWal(pBt->pPager) ){ + rc = sqlite3PagerBeginConcurrent(pBt->pPager); + if( rc==SQLITE_OK && wrflag ){ + rc = btreePtrmapAllocate(pBt); + } + } +#endif + if( rc==SQLITE_OK && wrflag ){ /* This call makes sure that the pager has the correct number of ** open savepoints. If the second parameter is greater than 0 and @@ -3937,7 +3941,6 @@ static int btreeFixUnlocked(Btree *p){ Pgno nPage = btreePagecount(pBt); u32 nFree = get4byte(&p1[36]); - assert( sqlite3PagerIsConcurrent(pPager) ); assert( pBt->pMap ); rc = sqlite3PagerUpgradeSnapshot(pPager, pPage1->pDbPage); assert( p1==pPage1->aData ); @@ -4101,8 +4104,11 @@ static void btreeEndTransaction(Btree *p){ unlockBtreeIfUnused(pBt); } - /* If this was an CONCURRENT transaction, delete the pBt->pMap object */ + /* If this was an CONCURRENT transaction, delete the pBt->pMap object. + ** Also call PagerEndConcurrent() to ensure that the pager has discarded + ** the record of all pages read within the transaction. */ btreePtrmapDelete(pBt); + sqlite3PagerEndConcurrent(pBt->pPager); btreeIntegrity(p); } diff --git a/src/pager.c b/src/pager.c index 9e6c2a0b9c..7db7544552 100644 --- a/src/pager.c +++ b/src/pager.c @@ -1741,16 +1741,35 @@ static int addToSavepointBitvecs(Pager *pPager, Pgno pgno){ return rc; } +#ifdef SQLITE_ENABLE_CONCURRENT +int sqlite3PagerBeginConcurrent(Pager *pPager){ + int rc = SQLITE_OK; + if( pPager->pAllRead==0 ){ + pPager->pAllRead = sqlite3BitvecCreate(pPager->dbSize); + if( pPager->pAllRead==0 ){ + rc = SQLITE_NOMEM; + } + } + return rc; +} + +void sqlite3PagerEndConcurrent(Pager *pPager){ + sqlite3BitvecDestroy(pPager->pAllRead); + pPager->pAllRead = 0; +} + +int sqlite3PagerIsWal(Pager *pPager){ + return pPager->pWal!=0; +} +#endif + /* ** Free the Pager.pInJournal and Pager.pAllRead bitvec objects. */ static void pagerFreeBitvecs(Pager *pPager){ sqlite3BitvecDestroy(pPager->pInJournal); pPager->pInJournal = 0; -#ifdef SQLITE_ENABLE_CONCURRENT - sqlite3BitvecDestroy(pPager->pAllRead); - pPager->pAllRead = 0; -#endif + sqlite3PagerEndConcurrent(pPager); } /* @@ -5612,10 +5631,6 @@ int sqlite3PagerBegin(Pager *pPager, int exFlag, int subjInMemory){ if( ALWAYS(pPager->eState==PAGER_READER) ){ assert( pPager->pInJournal==0 ); -#ifdef SQLITE_ENABLE_CONCURRENT - assert( pPager->pAllRead==0 ); -#endif - if( pagerUseWal(pPager) ){ /* If the pager is configured to use locking_mode=exclusive, and an ** exclusive lock on the database is not already held, obtain it now. @@ -5631,17 +5646,8 @@ int sqlite3PagerBegin(Pager *pPager, int exFlag, int subjInMemory){ /* Grab the write lock on the log file. If successful, upgrade to ** PAGER_RESERVED state. Otherwise, return an error code to the caller. ** The busy-handler is not invoked if another connection already - ** holds the write-lock. If possible, the upper layer will call it. - */ -#ifdef SQLITE_ENABLE_CONCURRENT - if( exFlag<0 ){ - pPager->pAllRead = sqlite3BitvecCreate(pPager->dbSize); - if( pPager->pAllRead==0 ){ - rc = SQLITE_NOMEM; - } - }else -#endif - { + ** holds the write-lock. If possible, the upper layer will call it. */ + if( exFlag>=0 ){ rc = sqlite3WalBeginWriteTransaction(pPager->pWal); } }else{ @@ -6184,13 +6190,6 @@ void sqlite3PagerSetDbsize(Pager *pPager, Pgno nSz){ pPager->dbSize = nSz; } -/* -** Return true if this pager is currently within an CONCURRENT transaction. -*/ -int sqlite3PagerIsConcurrent(Pager *pPager){ - return pPager->pAllRead!=0; -} - /* ** If this is a WAL mode connection and the WRITER lock is currently held, ** relinquish it. diff --git a/src/pager.h b/src/pager.h index c9ebaf1116..1df91bbca0 100644 --- a/src/pager.h +++ b/src/pager.h @@ -194,11 +194,17 @@ void sqlite3PagerTruncateImage(Pager*,Pgno); void sqlite3PagerRekey(DbPage*, Pgno, u16); +#ifdef SQLITE_ENABLE_CONCURRENT +void sqlite3PagerEndConcurrent(Pager*); +int sqlite3PagerBeginConcurrent(Pager*); void sqlite3PagerDropExclusiveLock(Pager*); -int sqlite3PagerIsConcurrent(Pager*); -int sqlite3PagerIswriteable(DbPage*); int sqlite3PagerUpgradeSnapshot(Pager *pPager, DbPage*); void sqlite3PagerSetDbsize(Pager *pPager, Pgno); +#else +# define sqlite3PagerEndConcurrent(x) +#endif + +int sqlite3PagerIswriteable(DbPage*); #if defined(SQLITE_HAS_CODEC) && !defined(SQLITE_OMIT_WAL) void *sqlite3PagerCodec(DbPage *); diff --git a/test/concurrent2.test b/test/concurrent2.test index 7f04a06b31..8650caea20 100644 --- a/test/concurrent2.test +++ b/test/concurrent2.test @@ -427,6 +427,31 @@ do_multiclient_test tn { } {0 {}} } +do_multiclient_test tn { + do_test 10.$tn.1 { + sql1 { + PRAGMA journal_mode = wal; + CREATE TABLE t1(a); + CREATE TABLE t2(b); + INSERT INTO t1 VALUES(1), (2), (3); + INSERT INTO t2 VALUES(1), (2), (3); + } + } {wal} + + do_test 10.$tn.2 { + sql1 { + BEGIN CONCURRENT; + SELECT * FROM t1; + INSERT INTO t2 VALUES(4); + } + } {1 2 3} + + do_test 10.$tn.3 { + sql2 { INSERT INTO t1 VALUES(4) } + list [catch {sql1 COMMIT} msg] $msg + } {1 {database is locked}} +} + finish_test