From 70af25d03de9e183ee914cceedbdfbe5feb7bd7c Mon Sep 17 00:00:00 2001 From: dan Date: Fri, 21 Aug 2015 17:57:16 +0000 Subject: [PATCH] Fix a problem with UNLOCKED transactions that free pages allocated within the same transaction. FossilOrigin-Name: 227bb8a1815c4dc6084970f06b0a6bfccdff3fd2 --- manifest | 14 +++++----- manifest.uuid | 2 +- src/btree.c | 62 ++++++++++++++++++++++++++++----------------- test/unlocked2.test | 34 +++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 31 deletions(-) diff --git a/manifest b/manifest index f5c3e2811a..ed83b7c162 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Merge\strunk\schanges\swith\sthis\sbranch. -D 2015-08-21T16:22:04.553 +C Fix\sa\sproblem\swith\sUNLOCKED\stransactions\sthat\sfree\spages\sallocated\swithin\sthe\ssame\stransaction. +D 2015-08-21T17:57:16.096 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in 4f663b6b4954b9b1eb0e6f08387688a93b57542d F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -278,7 +278,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 a00a7c4809d642bf6c63979e73face9fed53dc66 +F src/btree.c fca7884ea849190eff530f358f96c7304fcd2595 F src/btree.h 00d4cdb747c4172a5566faf037116985dbbc377e F src/btreeInt.h 1f7258e1f35ba3cc2197f9019f523dd20d59a0d8 F src/build.c e47b6fffe14a28d9050e6747beebb01597d37542 @@ -1226,7 +1226,7 @@ F test/unique.test 93f8b2ef5ea51b9495f8d6493429b1fd0f465264 F test/unique2.test 41e7f83c6827605991160a31380148a9fc5f1339 F test/unixexcl.test cd6c765f75e50e8e2c2ba763149e5d340ea19825 F test/unlocked.test d143a871bd874311d4e5c98ce458218ca6b579fd -F test/unlocked2.test aca373d7639fd11c946a8e1a78c57bab0f18bc8e +F test/unlocked2.test 4861218039ca2481613d7e6a999226185222c93a F test/unordered.test ca7adce0419e4ca0c50f039885e76ed2c531eda8 F test/update.test 6c68446b8a0a33d522a7c72b320934596a2d7d32 F test/uri.test 23662b7b61958b0f0e47082de7d06341ccf85d5b @@ -1378,7 +1378,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 82cd837e72ed4cf5821be717369694be29a2997e 7b8d17dd840f64dac9a018a4547a97de799e94ab -R a6aada1e15ca09028aeff07fdb262305 +P deaf3b18569025d8d168da29fae76142cfffe9e5 +R f09b37f1aebb9ccbb19d5d48d89374eb U dan -Z 957a88b73b652ebea177f80c25ee0401 +Z 1ea7f2e6e188cedc2dae0825d7ceb725 diff --git a/manifest.uuid b/manifest.uuid index ee7b5e2772..4ee4bbbd4b 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -deaf3b18569025d8d168da29fae76142cfffe9e5 \ No newline at end of file +227bb8a1815c4dc6084970f06b0a6bfccdff3fd2 \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index cc97fd8623..0bd34c7439 100644 --- a/src/btree.c +++ b/src/btree.c @@ -3870,7 +3870,7 @@ static int btreeFixUnlocked(Btree *p){ if( rc==SQLITE_OK ){ Pgno nHPage = get4byte(&p1[28]); - Pgno nFinal = nHPage; + Pgno nFinal = nHPage; /* Size of db after transaction merge */ if( sqlite3PagerIswriteable(pPage1->pDbPage) ){ Pgno iHTrunk = get4byte(&p1[32]); @@ -3892,6 +3892,9 @@ static int btreeFixUnlocked(Btree *p){ } if( nHPageiFirst through @@ -3902,22 +3905,28 @@ static int btreeFixUnlocked(Btree *p){ Pgno iLast = MIN(nPage, nHPage); /* Last page to move */ Pgno iPg; - nFinal = MAX(nPage, nHPage); /* Final size of database */ + nFinal = MAX(nPage, nHPage); for(iPg=pMap->iFirst; iPg<=iLast && rc==SQLITE_OK; iPg++){ MemPage *pPg = 0; Pgno iNew; /* New page number for pPg */ PtrmapEntry *pEntry; /* Pointer map entry for page iPg */ - btreeGetPage(pBt, iPg, &pPg, 0); - assert( sqlite3PagerIswriteable(pPg->pDbPage) ); - assert( sqlite3PagerPageRefcount(pPg->pDbPage)==1 ); pEntry = &pMap->aPtr[iPg - pMap->iFirst]; - iNew = ++nFinal; - - rc = relocatePage(pBt, pPg, pEntry->eType, pEntry->parent, iNew, 1); - releasePageNotNull(pPg); + if( pEntry->eType==PTRMAP_FREEPAGE ){ + MemPage *pFree = 0; + Pgno dummy; + rc = allocateBtreePage(pBt, &pFree, &dummy, iPg, BTALLOC_EXACT); + releasePage(pFree); + assert( rc!=SQLITE_OK || dummy==iPg ); + }else{ + btreeGetPage(pBt, iPg, &pPg, 0); + assert( sqlite3PagerIswriteable(pPg->pDbPage) ); + assert( sqlite3PagerPageRefcount(pPg->pDbPage)==1 ); + iNew = ++nFinal; + rc = relocatePage(pBt, pPg, pEntry->eType, pEntry->parent, iNew, 1); + releasePageNotNull(pPg); + } } - put4byte(&p1[28], nFinal); } } @@ -5775,14 +5784,16 @@ static int allocateBtreePage( Pgno mxPage; /* Total size of the database file */ assert( sqlite3_mutex_held(pBt->mutex) ); - assert( eMode==BTALLOC_ANY || (nearby>0 && IfNotOmitAV(pBt->autoVacuum)) ); + assert( eMode==BTALLOC_ANY || (nearby>0 && ISAUTOVACUUM) + || (eMode==BTALLOC_EXACT && sqlite3PagerIsUnlocked(pBt->pPager)) + ); pPage1 = pBt->pPage1; mxPage = btreePagecount(pBt); /* EVIDENCE-OF: R-05119-02637 The 4-byte big-endian integer at offset 36 ** stores stores the total number of pages on the freelist. */ n = get4byte(&pPage1->aData[36]); testcase( n==mxPage-1 ); - if( n>=mxPage ){ + if( sqlite3PagerIsUnlocked(pBt->pPager)==0 && n>=mxPage ){ return SQLITE_CORRUPT_BKPT; } @@ -5791,7 +5802,6 @@ static int allocateBtreePage( ** of these operations involve modifying page 1 header fields, page 1 ** will definitely be written by this transaction. If this is an UNLOCKED ** transaction, ensure the BtreePtrmap structure has been allocated. */ - assert( eMode!=BTALLOC_EXACT || sqlite3PagerIsUnlocked(pBt->pPager)==0 ); rc = sqlite3PagerWrite(pPage1->pDbPage); if( rc ) return rc; @@ -5805,19 +5815,25 @@ static int allocateBtreePage( ** shows that the page 'nearby' is somewhere on the free-list, then ** the entire-list will be searched for that page. */ -#ifndef SQLITE_OMIT_AUTOVACUUM if( eMode==BTALLOC_EXACT ){ - if( nearby<=mxPage ){ - u8 eType; - assert( nearby>0 ); - assert( pBt->autoVacuum ); - rc = ptrmapGet(pBt, nearby, &eType, 0); - if( rc ) return rc; - if( eType==PTRMAP_FREEPAGE ){ - searchList = 1; + assert( ISAUTOVACUUM==!sqlite3PagerIsUnlocked(pBt->pPager) ); + if( ISAUTOVACUUM ){ + if( nearby<=mxPage ){ + u8 eType; + assert( nearby>0 ); + assert( pBt->autoVacuum ); + rc = ptrmapGet(pBt, nearby, &eType, 0); + if( rc ) return rc; + if( eType==PTRMAP_FREEPAGE ){ + searchList = 1; + } } + }else{ + searchList = 1; } - }else if( eMode==BTALLOC_LE ){ + } +#ifndef SQLITE_OMIT_AUTOVACUUM + else if( eMode==BTALLOC_LE ){ searchList = 1; } #endif diff --git a/test/unlocked2.test b/test/unlocked2.test index 1ec9050b60..42742111c8 100644 --- a/test/unlocked2.test +++ b/test/unlocked2.test @@ -187,5 +187,39 @@ do_execsql_test 3.5.2 { } {2} do_execsql_test 3.5.3 { PRAGMA integrity_check } {ok} +#------------------------------------------------------------------------- +# Test that nothing goes wrong if an UNLOCKED transaction allocates a +# page at the end of the file, frees it within the same transaction, and +# then has to move the same page to avoid a conflict on COMMIT. +# +do_multiclient_test tn { + do_test 4.$tn.1 { + sql1 { + PRAGMA journal_mode = wal; + CREATE TABLE t1(x); + CREATE TABLE t2(x); + } + } {wal} + + do_test 4.$tn.2 { + sql1 { + BEGIN UNLOCKED; + INSERT INTO t1 VALUES(randomblob(1500)); + INSERT INTO t1 VALUES(randomblob(1500)); + DELETE FROM t1 WHERE rowid = 1; + } + + sql2 { + INSERT INTO t2 VALUES(randomblob(1500)); + INSERT INTO t2 VALUES(randomblob(1500)); + INSERT INTO t2 VALUES(randomblob(1500)); + INSERT INTO t2 VALUES(randomblob(1500)); + DELETE FROM t2 WHERE rowid IN (1, 2); + } + + sql1 COMMIT + } {} +} + finish_test -- 2.47.3