diff --git a/manifest b/manifest index a92f8a6ff1..e571284281 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\sspacing\stypo\sin\scomment.\s\sNo\schanges\sto\scode. -D 2015-12-10T03:16:47.044 +C Have\ssqlite3_snapshot_open()\savoid\sa\srace\scondition\sby\staking\sa\sshared\sCHECKPOINTER\slock\swhile\schecking\spInfo->nBackfillAttempted. +D 2015-12-10T15:45:15.186 F Makefile.in 28bcd6149e050dff35d4dcfd97e890cd387a499d F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434 F Makefile.msc e8fdca1cb89a1b58b5f4d3a130ea9a3d28cb314d @@ -415,7 +415,7 @@ F src/vdbesort.c a7ec02da4494c59dfd071126dd3726be5a11459d F src/vdbetrace.c 8befe829faff6d9e6f6e4dee5a7d3f85cc85f1a0 F src/vtab.c 2a8b44aa372c33f6154208e7a7f6c44254549806 F src/vxworks.h c18586c8edc1bddbc15c004fa16aeb1e1342b4fb -F src/wal.c 964af61902acead7de6e95035d0ce597f7019da9 +F src/wal.c 32ee7dc4b689321d1650fba2e937ddc3bccfb06f F src/wal.h 907943dfdef10b583e81906679a347e0ec6f1b1b F src/walker.c 2e14d17f592d176b6dc879c33fbdec4fbccaa2ba F src/where.c b18edbb9e5afabb77f4f27550c471c5c824e0fe7 @@ -1020,7 +1020,7 @@ F test/skipscan2.test d1d1450952b7275f0b0a3a981f0230532743951a F test/skipscan3.test ec5bab3f81c7038b43450e7b3062e04a198bdbb5 F test/skipscan5.test 67817a4b6857c47e0e33ba3e506da6f23ef68de2 F test/skipscan6.test 5866039d03a56f5bd0b3d172a012074a1d90a15b -F test/snapshot.test 800e0be4488acb88dd38ff9e9b83edb71d9d5a9d +F test/snapshot.test f91d907460e7acc01d531834d068e1215ccac7e4 F test/soak.test 0b5b6375c9f4110c828070b826b3b4b0bb65cd5f F test/softheap1.test 843cd84db9891b2d01b9ab64cef3e9020f98d087 F test/sort.test 3f492e5b7be1d3f756728d2ff6edf4f6091e84cb @@ -1409,7 +1409,7 @@ F tool/vdbe_profile.tcl 246d0da094856d72d2c12efec03250d71639d19f F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4 F tool/warnings.sh 48bd54594752d5be3337f12c72f28d2080cb630b F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f -P cb68e9d0738fc7db7316947b4d2aab91aae819f2 -R 76ea636367cbd8afb53a2670ba03ae31 -U mistachkin -Z 1d7611d28a33701f209bc754328c05f3 +P 3a18526fc2253658dad84c5e600481c8a62efe40 +R 7bb35c02cb12ea2ff304c777229238a5 +U dan +Z 0b9f17bd41106b46c64f60d98c85b1e3 diff --git a/manifest.uuid b/manifest.uuid index d5273835aa..825f8d1181 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -3a18526fc2253658dad84c5e600481c8a62efe40 \ No newline at end of file +8084eae0bc4f6513b1147fb890a6b2813f1c0a09 \ No newline at end of file diff --git a/src/wal.c b/src/wal.c index 0115e06027..0439ed4253 100644 --- a/src/wal.c +++ b/src/wal.c @@ -1217,7 +1217,7 @@ finished: */ pInfo = walCkptInfo(pWal); pInfo->nBackfill = 0; - pInfo->nBackfillAttempted = 0; + pInfo->nBackfillAttempted = pWal->hdr.mxFrame; pInfo->aReadMark[0] = 0; for(i=1; iaReadMark[i] = READMARK_NOT_USED; if( pWal->hdr.mxFrame ) pInfo->aReadMark[1] = pWal->hdr.mxFrame; @@ -1757,7 +1757,6 @@ static int walCheckpoint( ** cannot be backfilled from the WAL. */ mxSafeFrame = pWal->hdr.mxFrame; - pInfo->nBackfillAttempted = mxSafeFrame; mxPage = pWal->hdr.nPage; for(i=1; inBackfill; + pInfo->nBackfillAttempted = mxSafeFrame; + /* Sync the WAL to disk */ if( sync_flags ){ rc = sqlite3OsSync(pWal->pWalFd, sync_flags); @@ -2296,9 +2297,6 @@ static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int cnt){ } if( (pWal->readOnly & WAL_SHM_RDONLY)==0 && (mxReadMarkpSnapshot==0 -#endif ){ for(i=1; ipSnapshot ) return SQLITE_BUSY_SNAPSHOT; -#endif assert( rc==SQLITE_BUSY || (pWal->readOnly & WAL_SHM_RDONLY)!=0 ); return rc==SQLITE_BUSY ? WAL_RETRY : SQLITE_READONLY_CANTLOCK; } @@ -2410,9 +2405,10 @@ int sqlite3WalBeginReadTransaction(Wal *pWal, int *pChanged){ if( rc==SQLITE_OK ){ if( pSnapshot && memcmp(pSnapshot, &pWal->hdr, sizeof(WalIndexHdr))!=0 ){ /* At this point the client has a lock on an aReadMark[] slot holding - ** a value equal to or smaller than pSnapshot->mxFrame. Verify that - ** pSnapshot is still valid before continuing. Reasons why pSnapshot - ** might no longer be valid: + ** a value equal to or smaller than pSnapshot->mxFrame, but pWal->hdr + ** is populated with the wal-index header corresponding to the head + ** of the wal file. Verify that pSnapshot is still valid before + ** continuing. Reasons why pSnapshot might no longer be valid: ** ** (1) The WAL file has been reset since the snapshot was taken. ** In this case, the salt will have changed. @@ -2426,10 +2422,22 @@ int sqlite3WalBeginReadTransaction(Wal *pWal, int *pChanged){ assert( pWal->readLock>0 ); assert( pInfo->aReadMark[pWal->readLock]<=pSnapshot->mxFrame ); - /* Check that the wal file has not been wrapped. Assuming it has not, - ** overwrite pWal->hdr with *pSnapshot and set *pChanged as appropriate - ** for opening the snapshot. Or, if the wal file has been wrapped - ** since pSnapshot was written, return SQLITE_BUSY_SNAPSHOT. */ + /* It is possible that there is a checkpointer thread running + ** concurrent with this code. If this is the case, it may be that the + ** checkpointer has already determined that it will checkpoint + ** snapshot X, where X is later in the wal file than pSnapshot, but + ** has not yet set the pInfo->nBackfillAttempted variable to indicate + ** its intent. To avoid the race condition this leads to, ensure that + ** there is no checkpointer process by taking a shared CKPT lock + ** before checking pInfo->nBackfillAttempted. */ + rc = walLockShared(pWal, WAL_CKPT_LOCK); + + /* Check that the wal file has not been wrapped. Assuming that it has + ** not, also check that no checkpointer has attempted to checkpoint + ** any frames beyond pSnapshot->mxFrame. If either of these conditions + ** are true, return SQLTIE_BUSY_SNAPSHOT. Otherwise, overwrite pWal->hdr + ** with *pSnapshot and set *pChanged as appropriate for opening the + ** snapshot. */ if( memcmp(pSnapshot->aSalt, pWal->hdr.aSalt, sizeof(pWal->hdr.aSalt))==0 && pSnapshot->mxFrame>=pInfo->nBackfillAttempted ){ @@ -2439,6 +2447,9 @@ int sqlite3WalBeginReadTransaction(Wal *pWal, int *pChanged){ rc = SQLITE_BUSY_SNAPSHOT; } + /* Release the shared CKPT lock obtained above. */ + walUnlockShared(pWal, WAL_CKPT_LOCK); + if( rc!=SQLITE_OK ){ sqlite3WalEndReadTransaction(pWal); } diff --git a/test/snapshot.test b/test/snapshot.test index 5d3758c010..965e10b683 100644 --- a/test/snapshot.test +++ b/test/snapshot.test @@ -57,8 +57,6 @@ do_execsql_test 1.3.2 COMMIT # Check that a simple case works. Reuse the database created by the # block of tests above. # -# UPDATE: This case (2.1) no longer works. 2.2 does. -# do_execsql_test 2.1.0 { BEGIN; SELECT * FROM t1; @@ -75,8 +73,11 @@ do_test 2.1.1 { do_test 2.1.2 { execsql BEGIN - list [catch { sqlite3_snapshot_open db main $snapshot } msg] $msg -} {1 SQLITE_BUSY_SNAPSHOT} + sqlite3_snapshot_open db main $snapshot + execsql { + SELECT * FROM t1; + } +} {1 2 3 4 5 6 7 8} do_test 2.1.3 { sqlite3_snapshot_free $snapshot @@ -99,7 +100,7 @@ do_test 2.2.1 { } } {1 2 3 4 5 6 7 8 9 10 11 12} -do_test 2.1.2 { +do_test 2.2.2 { execsql BEGIN sqlite3_snapshot_open db main $snapshot execsql { @@ -107,7 +108,7 @@ do_test 2.1.2 { } } {1 2 3 4 5 6 7 8 9 10} -do_test 2.1.3 { +do_test 2.2.3 { sqlite3_snapshot_free $snapshot execsql COMMIT execsql COMMIT db2