diff --git a/manifest b/manifest index e273be4a1d..7b1ca4d540 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Mark\sa\scondition\sin\swal.c\sas\sALWAYS(). -D 2010-06-05T14:42:58 +C Remove\sa\scondition\sfrom\ssqlite3WalRead()\sthat\sis\sunreachable\sas\sof\sthe\schanges\sto\sclear\sentries\sout\sof\sthe\swal-index\shash\stables\son\stransaction\sor\ssavepoint\srollback. +D 2010-06-05T18:12:24 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in a5cad1f8f3e021356bfcc6c77dc16f6f1952bbc3 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -224,7 +224,7 @@ F src/vdbeblob.c 5327132a42a91e8b7acfb60b9d2c3b1c5c863e0e F src/vdbemem.c 2a82f455f6ca6f78b59fb312f96054c04ae0ead1 F src/vdbetrace.c 864cef96919323482ebd9986f2132435115e9cc2 F src/vtab.c a0f8a40274e4261696ef57aa806de2776ab72cda -F src/wal.c 3a448ad3a563b7b9fe6982e69f56fab327eda196 +F src/wal.c 22a58cdfc167d29353480f1a587d844997120518 F src/wal.h 4ace25262452d17e7d3ec970c89ee17794004008 F src/walker.c 3112bb3afe1d85dc52317cb1d752055e9a781f8f F src/where.c 75fee9e255b62f773fcadd1d1f25b6f63ac7a356 @@ -763,7 +763,7 @@ F test/vtab_err.test 0d4d8eb4def1d053ac7c5050df3024fd47a3fbd8 F test/vtab_shared.test 0eff9ce4f19facbe0a3e693f6c14b80711a4222d F test/wal.test bfec61450b47cdf09f7d2269f9e9967683b8b0fc F test/wal2.test 743d9b86041e57ba986dd7e3891c67725f9e2b2b -F test/wal3.test 91243bd815e35cfda977df071f95ee2be3cfe236 +F test/wal3.test 1c5e9535e0e307f837e059ec45842e7101796a96 F test/wal_common.tcl 3e953ae60919281688ea73e4d0aa0e1bc94becd9 F test/walbak.test e7650a26eb4b8abeca9b145b1af1e63026dde432 F test/walcksum.test 4efa8fb88c32bed8288ea4385a9cc113a5c8f0bf @@ -817,7 +817,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f -P f9d4ae0e8cc5d32c52eb78799f7959dd236ea9de -R 354467ef49285b1e7f4721d72defda65 +P 3fe0cc784ac586358c08f87fba458dfbb5eec6f2 +R 2aa1b1d87a9dc475383f9885b8e6d33d U dan -Z 2159f852b96b4a4c0e2c6c207c85dab6 +Z e4cbf876bae12325e294a44d5e590e7b diff --git a/manifest.uuid b/manifest.uuid index fe64f89fa3..b1cde350af 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -3fe0cc784ac586358c08f87fba458dfbb5eec6f2 \ No newline at end of file +394204735a842b04b677cca20485b1578e475d4c \ No newline at end of file diff --git a/src/wal.c b/src/wal.c index dcff296793..15a9ecc16a 100644 --- a/src/wal.c +++ b/src/wal.c @@ -1746,7 +1746,20 @@ static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int cnt){ */ rc = walLockShared(pWal, WAL_READ_LOCK(0)); if( rc==SQLITE_OK ){ - if( pHdr->mxFrame!=pWal->hdr.mxFrame ){ + if( memcmp(pHdr, &pWal->hdr, sizeof(WalIndexHdr)) ){ + /* It is not safe to allow the reader to continue here if frames + ** may have been appended to the log before READ_LOCK(0) was obtained. + ** When holding READ_LOCK(0), the reader ignores the entire log file, + ** which implies that the database file contains a trustworthy + ** snapshoT. Since holding READ_LOCK(0) prevents a checkpoint from + ** happening, this is usually correct. + ** + ** However, if frames have been appended to the log (or if the log + ** is wrapped and written for that matter) before the READ_LOCK(0) + ** is obtained, that is not necessarily true. A checkpointer may + ** have started to backfill the appended frames but crashed before + ** it finished. Leaving a corrupt image in the database file. + */ walUnlockShared(pWal, WAL_READ_LOCK(0)); return WAL_RETRY; } @@ -1917,29 +1930,6 @@ int sqlite3WalRead( ** (iFrame<=iLast): ** This condition filters out entries that were added to the hash ** table after the current read-transaction had started. - ** - ** (iFrame>iRead): - ** This filters out a dangerous class of garbage data. The - ** garbage hash slot may refer to a frame with the correct page - ** number, but not the most recent version of the frame. For - ** example, if at the start of the read-transaction the WAL - ** contains three copies of the desired page in frames 2, 3 and 4, - ** the hash table may contain the following: - ** - ** { ..., 2, 3, 4, 99, 99, ..... } - ** - ** The correct answer is to read data from frame 4. But a - ** dirty-read may potentially cause the hash-table to appear as - ** follows to the reader: - ** - ** { ..., 2, 3, 4, 3, 99, ..... } - ** - ** Without this part of the if(...) clause, the reader might - ** incorrectly read data from frame 3 instead of 4. This would be - ** an error. - ** - ** It is not actually clear to the developers that such a dirty-read - ** can occur. But if it does, it should not cause any problems. */ for(iHash=iLast; iHash>0 && iRead==0; iHash-=HASHTABLE_NPAGE){ volatile HASHTABLE_DATATYPE *aHash; /* Pointer to hash table */ @@ -1953,7 +1943,8 @@ int sqlite3WalRead( if( mxHash > HASHTABLE_NPAGE ) mxHash = HASHTABLE_NPAGE; for(iKey=walHash(pgno); aHash[iKey]; iKey=walNextHash(iKey)){ u32 iFrame = aHash[iKey] + iZero; - if( iFrame<=iLast && aPgno[iFrame]==pgno && iFrame>iRead ){ + if( iFrame<=iLast && aPgno[iFrame]==pgno ){ + assert( iFrame>iRead ); iRead = iFrame; } } diff --git a/test/wal3.test b/test/wal3.test index 56eae516cc..430855d913 100644 --- a/test/wal3.test +++ b/test/wal3.test @@ -418,8 +418,151 @@ db close T delete #------------------------------------------------------------------------- -# When opening a read-transaction on a database +# When opening a read-transaction on a database, if the entire log has +# already been copied to the database file, the reader grabs a special +# kind of read lock (on aReadMark[0]). This test case tests the outcome +# of the following: # +# + The reader discovering that between the time when it determined +# that the log had been completely backfilled and the lock is obtained +# that a writer has written to the log. In this case the reader should +# acquire a different read-lock (not aReadMark[0]) and read the new +# snapshot. +# +# + The attempt to obtain the lock on aReadMark[0] fails with SQLITE_BUSY. +# This can happen if a checkpoint is ongoing. In this case also simply +# obtain a different read-lock. +# +catch {db close} +testvfs T -default 1 +do_test wal3-6.1.1 { + file delete -force test.db test.db-journal test.db wal + sqlite3 db test.db + execsql { PRAGMA journal_mode = WAL } + execsql { + CREATE TABLE t1(a, b); + INSERT INTO t1 VALUES('o', 't'); + INSERT INTO t1 VALUES('t', 'f'); + } +} {} +do_test wal3-6.1.2 { + sqlite3 db2 test.db + sqlite3 db3 test.db + execsql { BEGIN ; SELECT * FROM t1 } db3 +} {o t t f} +do_test wal3-6.1.3 { + execsql { PRAGMA wal_checkpoint } db2 +} {} + +# At this point the log file has been fully checkpointed. However, +# connection [db3] holds a lock that prevents the log from being wrapped. +# Test case 3.6.1.4 has [db] attempt a read-lock on aReadMark[0]. But +# as it is obtaining the lock, [db2] appends to the log file. +# +T filter xShmLock +T script lock_callback +proc lock_callback {method file handle spec} { + if {$spec == "3 1 lock shared"} { + # This is the callback for [db] to obtain the read lock on aReadMark[0]. + # Disable future callbacks using [T filter {}] and write to the log + # file using [db2]. [db3] is preventing [db2] from wrapping the log + # here, so this is an append. + T filter {} + db2 eval { INSERT INTO t1 VALUES('f', 's') } + } + return SQLITE_OK +} +do_test wal3-6.1.4 { + execsql { + BEGIN; + SELECT * FROM t1; + } +} {o t t f f s} + +# [db] should be left holding a read-lock on some slot other than +# aReadMark[0]. Test this by demonstrating that the read-lock is preventing +# the log from being wrapped. +# +do_test wal3-6.1.5 { + db3 eval COMMIT + db2 eval { PRAGMA wal_checkpoint } + set sz1 [file size test.db-wal] + db2 eval { INSERT INTO t1 VALUES('s', 'e') } + set sz2 [file size test.db-wal] + expr {$sz2>$sz1} +} {1} + +# Test that if [db2] had not interfered when [db] was trying to grab +# aReadMark[0], it would have been possible to wrap the log in 3.6.1.5. +# +do_test wal3-6.1.6 { + execsql { COMMIT } + execsql { PRAGMA wal_checkpoint } db2 + execsql { + BEGIN; + SELECT * FROM t1; + } +} {o t t f f s s e} +do_test wal3-6.1.7 { + db2 eval { PRAGMA wal_checkpoint } + set sz1 [file size test.db-wal] + db2 eval { INSERT INTO t1 VALUES('n', 't') } + set sz2 [file size test.db-wal] + expr {$sz2==$sz1} +} {1} + +db3 close +db2 close +db close + +do_test wal3-6.2.1 { + file delete -force test.db test.db-journal test.db wal + sqlite3 db test.db + sqlite3 db2 test.db + execsql { PRAGMA journal_mode = WAL } + execsql { + CREATE TABLE t1(a, b); + INSERT INTO t1 VALUES('h', 'h'); + INSERT INTO t1 VALUES('l', 'b'); + } +} {} + +T filter xShmLock +T script lock_callback +proc lock_callback {method file handle spec} { + if {$spec == "3 1 unlock exclusive"} { + T filter {} + set ::R [db2 eval { + BEGIN; + SELECT * FROM t1; + }] + } +} +do_test wal3-6.2.2 { + execsql { PRAGMA wal_checkpoint } +} {} +do_test wal3-6.2.3 { + set ::R +} {h h l b} +do_test wal3-6.2.4 { + set sz1 [file size test.db-wal] + execsql { INSERT INTO t1 VALUES('b', 'c'); } + set sz2 [file size test.db-wal] + expr {$sz2 > $sz1} +} {1} +do_test wal3-6.2.5 { + db2 eval { COMMIT } + execsql { PRAGMA wal_checkpoint } + set sz1 [file size test.db-wal] + execsql { INSERT INTO t1 VALUES('n', 'o'); } + set sz2 [file size test.db-wal] + expr {$sz2 == $sz1} +} {1} + +db2 close +db close +T delete + finish_test