From db3403973c5502ca99cf24d52e97fc70b83f1d64 Mon Sep 17 00:00:00 2001 From: danielk1977 Date: Fri, 16 Jan 2009 16:40:14 +0000 Subject: [PATCH] Fix a change-counter bug similar to #3584. This one is much more obscure though, requiring a transient IO or malloc error to occur while running in exclusive mode. (CVS 6189) FossilOrigin-Name: 9f07d2d9226b73c4dc311fd142e0aaba4ffcb078 --- manifest | 14 +++++----- manifest.uuid | 2 +- src/pager.c | 13 ++++++++- test/malloc.test | 68 +++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 87 insertions(+), 10 deletions(-) diff --git a/manifest b/manifest index e47498fffa..423bfba73f 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Revert\s(6187).\s(CVS\s6188) -D 2009-01-16T16:23:38 +C Fix\sa\schange-counter\sbug\ssimilar\sto\s#3584.\sThis\sone\sis\smuch\smore\sobscure\sthough,\srequiring\sa\stransient\sIO\sor\smalloc\serror\sto\soccur\swhile\srunning\sin\sexclusive\smode.\s(CVS\s6189) +D 2009-01-16T16:40:14 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in 8a00230c566c1a1cfc7ae53eedd458b32034da30 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -142,7 +142,7 @@ F src/os_common.h 24525d8b7bce66c374dfc1810a6c9043f3359b60 F src/os_os2.c bed77dc26e3a95ce4a204936b9a1ca6fe612fcc5 F src/os_unix.c c0ebce13fac2db7900367e83a3ebbd112ea4e90e F src/os_win.c 496e3ceb499aedc63622a89ef76f7af2dd902709 -F src/pager.c b99d1e9f9c971cf42f4c7ede17117f238e9b9c2b +F src/pager.c e8e2ae1ef6a20464a627d29fd4ba29461b7632a5 F src/pager.h 3345547d4b5b4db323f50d855d91a01837c7f2de F src/parse.y b214295a91e985c42adb6bfd3ad1c56c47828e8d F src/pcache.c a3c729f4bb3464fab27617ab7411916e0cded2bf @@ -430,7 +430,7 @@ F test/lock6.test eafa70db6f50b6f6291f4f83b80e98834724a50d F test/lookaside.test e69f822f13745f1d5c445c6e30e30f059f30c8e5 F test/main.test 187a9a1b5248ed74a83838c581c15ec6023b555b F test/make-where7.tcl 40bb740b37eead343eaf57b74ab72d2a5a304745 -F test/malloc.test 95e94f8a3dd1169c8774788ec4148ea032ea20d8 +F test/malloc.test 228840643d2a4bc536bc8f677375948db62dbe1f F test/malloc3.test 4bc57f850b212f706f3e1b37c4eced1d5a727cd1 F test/malloc4.test 957337613002b7058a85116493a262f679f3a261 F test/malloc5.test 20d1a0884b03edf811bfd7005faade028367e7c8 @@ -697,7 +697,7 @@ F tool/speedtest16.c c8a9c793df96db7e4933f0852abb7a03d48f2e81 F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e -P aa67fd0cdb4f53a0c6e15c001d37554d15006718 -R c8509fb836bf1d76f87b59b07c81ef45 +P a353c1ab376b159c4d12532412365318cdbdcc60 +R 0bdccace0ceb1b72a77c9b992ad652ac U danielk1977 -Z 78ca002c1238dcdf491f58ff6219cf73 +Z 007bd5d387bc84161ee29cd1e4862ae7 diff --git a/manifest.uuid b/manifest.uuid index 516cee256a..b153e2afc1 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -a353c1ab376b159c4d12532412365318cdbdcc60 \ No newline at end of file +9f07d2d9226b73c4dc311fd142e0aaba4ffcb078 \ No newline at end of file diff --git a/src/pager.c b/src/pager.c index 0df2b594fc..32a04f1d14 100644 --- a/src/pager.c +++ b/src/pager.c @@ -18,7 +18,7 @@ ** file simultaneously, or one process from reading the database while ** another is writing. ** -** @(#) $Id: pager.c,v 1.553 2009/01/16 16:23:38 danielk1977 Exp $ +** @(#) $Id: pager.c,v 1.554 2009/01/16 16:40:14 danielk1977 Exp $ */ #ifndef SQLITE_OMIT_DISKIO #include "sqliteInt.h" @@ -1733,6 +1733,17 @@ end_playback: sqlite3OsFileControl(pPager->fd,SQLITE_FCNTL_DB_UNCHANGED,0)>=SQLITE_OK ); + /* If this playback is happening automatically as a result of an IO or + ** malloc error that occured after the change-counter was updated but + ** before the transaction was committed, then the change-counter + ** modification may just have been reverted. If this happens in exclusive + ** mode, then subsequent transactions performed by the connection will not + ** update the change-counter at all. This may lead to cache inconsistency + ** problems for other processes at some point in the future. So, just + ** in case this has happened, clear the changeCountDone flag now. + */ + pPager->changeCountDone = 0; + if( rc==SQLITE_OK ){ zMaster = pPager->pTmpSpace; rc = readMasterJournal(pPager->jfd, zMaster, pPager->pVfs->mxPathname+1); diff --git a/test/malloc.test b/test/malloc.test index ddd6c2eaf3..b9af7426c0 100644 --- a/test/malloc.test +++ b/test/malloc.test @@ -16,7 +16,7 @@ # to see what happens in the library if a malloc were to really fail # due to an out-of-memory situation. # -# $Id: malloc.test,v 1.73 2009/01/10 16:15:09 danielk1977 Exp $ +# $Id: malloc.test,v 1.74 2009/01/16 16:40:14 danielk1977 Exp $ set testdir [file dirname $argv0] source $testdir/tester.tcl @@ -721,6 +721,72 @@ do_malloc_test 31 -sqlprep { INSERT INTO t1 VALUES(1, 2); } +# When written, this test provoked an obscure change-counter bug. +# +# If, when running in exclusive mode, a malloc() failure occurs +# after the database file change-counter has been written but +# before the transaction has been committed, then the transaction +# is automatically rolled back. However, internally the +# Pager.changeCounterDone flag was being left set. This means +# that if the same connection attempts another transaction following +# the malloc failure and rollback, the change counter will not +# be updated. This could corrupt another processes cache. +# +do_malloc_test 32 -tclprep { + # Build a small database containing an indexed table. + # + db eval { + BEGIN; + CREATE TABLE t1(a PRIMARY KEY, b); + INSERT INTO t1 VALUES(1, 'one'); + INSERT INTO t1 VALUES(2, 'two'); + INSERT INTO t1 VALUES(3, 'three'); + COMMIT; + PRAGMA locking_mode = exclusive; + } + + # Open a second database connection. Load the table (but not index) + # into the second connections pager cache. + # + sqlite3 db2 test.db + db2 eval { SELECT b FROM t1 } + +} -tclbody { + # Running in exclusive mode, perform a database transaction that + # modifies both the database table and index. For iterations where + # the malloc failure occurs after updating the change counter but + # before committing the transaction, this should result in the + # transaction being rolled back but the changeCounterDone flag + # left set. + # + db eval { UPDATE t1 SET a = a + 3 } +} -cleanup { + + # Perform another transaction using the first connection. Unlock + # the database after doing so. If this is one of the right iterations, + # then this should result in the database contents being updated but + # the change-counter left as it is. + # + db eval { + PRAGMA locking_mode = normal; + UPDATE t1 SET a = a + 3; + } + + # Now do an integrity check with the second connection. The second + # connection still has the database table in its cache. If this is + # one of the magic iterations and the change counter was not modified, + # then it won't realize that the cached data is out of date. Since + # the cached data won't match the up to date index data read from + # the database file, the integrity check should fail. + # + set zRepeat "transient" + if {$::iRepeat} {set zRepeat "persistent"} + do_test malloc-32.$zRepeat.${::n}.integrity { + execsql {PRAGMA integrity_check} db2 + } {ok} + db2 close +} + # Ensure that no file descriptors were leaked. do_test malloc-99.X { catch {db close}