Fix a couple of potential corruption problems in pager.c. (CVS 6143)

FossilOrigin-Name: 5a39525ba3e65f1c6df3cf23fbfb57f6a0bf4b62

manifest

@@ -1,5 +1,5 @@
-C Add\sa\stest\sscript\sfor\sticket\s#2565.\s\sChange\sthe\sassert()\sin\spager.c\sinto\na\stestcase()\smacro.\s(CVS\s6142)
+C Fix\sa\scouple\sof\spotential\scorruption\sproblems\sin\spager.c.\s(CVS\s6143)
-D 2009-01-08T15:24:02
+D 2009-01-08T17:50:46
 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0
 F Makefile.in 05461a9b5803d5ad10c79f989801e9fd2cc3e592
 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 a1f05f59c24e61186c981f2a7fea13986db620f1
 F src/os_win.c 496e3ceb499aedc63622a89ef76f7af2dd902709
-F src/pager.c 2b35462d94ef080eb21902ba4747593ef261069f
+F src/pager.c 35445d2172eb39c47ec146777c26e47e6d865538
 F src/pager.h 9870acb2d653848d90d765d7cbf163496d6c8111
 F src/parse.y 4d0e33a702dc3ea7b69d8ae1914b3fbd32e46057
 F src/pcache.c 16dc8da6e6ba6250f8dfd9ee46036db1cbceedc6
@@ -288,6 +288,7 @@ F test/crash4.test 02ff4f15c149ca1e88a5c299b4896c84d9450c3b
 F test/crash5.test 80a2f7073381837fc082435c97df52a830abcd80
 F test/crash6.test 9c730cf06335003cb1f5cfceddacd044155336e0
 F test/crash7.test e20a7b9ee1d16eaef7c94a4cb7ed2191b4d05970
+F test/crash8.test 8a7134b3b51e6914054ecd47115a62d9be6726da
 F test/crashtest1.c 09c1c7d728ccf4feb9e481671e29dda5669bbcc2
 F test/createtab.test 199cf68f44e5d9e87a0b8afc7130fdeb4def3272
 F test/cse.test 277350a26264495e86b1785f34d2d0c8600e021c
@@ -695,7 +696,7 @@ F tool/speedtest16.c c8a9c793df96db7e4933f0852abb7a03d48f2e81
 F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff
 F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224
 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e
-P 81014334ad57e380e21c47ad6eebe9f16b4ad24c
+P 1e53e382e5030de4d908098bf77af053dd81f0c5
-R 1fb9e89475372962c965501bf1119839
+R 124292e0acdcfa03ab956f6d28184270
-U drh
+U danielk1977
-Z f5bb78d1eaf6298f714991d532608b85
+Z 35376a8787a84eed4d896203061ddd89

manifest.uuid

@@ -1 +1 @@
-1e53e382e5030de4d908098bf77af053dd81f0c5
+5a39525ba3e65f1c6df3cf23fbfb57f6a0bf4b62

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.541 2009/01/08 15:24:02 drh Exp $
+** @(#) $Id: pager.c,v 1.542 2009/01/08 17:50:46 danielk1977 Exp $
 */
 #ifndef SQLITE_OMIT_DISKIO
 #include "sqliteInt.h"
@@ -580,7 +580,7 @@ static int readMasterJournal(sqlite3_file *pJrnl, char *zMaster, u32 nMaster){
 ** 2000                      2048
 ** 
 */
-static void seekJournalHdr(Pager *pPager){
+static i64 journalHdrOffset(Pager *pPager){
   i64 offset = 0;
   i64 c = pPager->journalOff;
   if( c ){
@@ -589,7 +589,10 @@ static void seekJournalHdr(Pager *pPager){
   assert( offset%JOURNAL_HDR_SZ(pPager)==0 );
   assert( offset>=c );
   assert( (offset-c)<JOURNAL_HDR_SZ(pPager) );
-  pPager->journalOff = offset;
+  return offset;
+}
+static void seekJournalHdr(Pager *pPager){
+  pPager->journalOff = journalHdrOffset(pPager);
 }
 
 /*
@@ -2487,6 +2490,35 @@ static int syncJournal(Pager *pPager){
       assert( pPager->journalOpen );
 
       if( 0==(iDc&SQLITE_IOCAP_SAFE_APPEND) ){
+        i64 jrnlOff = journalHdrOffset(pPager);
+        u8 zMagic[8];
+
+        /* This block deals with an obscure problem. If the last connection
+        ** that wrote to this database was operating in persistent-journal
+        ** mode, then the journal file may at this point actually be larger
+        ** than Pager.journalOff bytes. If the next thing in the journal
+        ** file happens to be a journal-header (written as part of the
+        ** previous connections transaction), and a crash or power-failure 
+        ** occurs after nRec is updated but before this connection writes 
+        ** anything else to the journal file (or commits/rolls back its 
+        ** transaction), then SQLite may become confused when doing the 
+        ** hot-journal rollback following recovery. It may roll back all
+        ** of this connections data, then proceed to rolling back the old,
+        ** out-of-date data that follows it. Database corruption.
+        **
+        ** To work around this, if the journal file does appear to contain
+        ** a valid header following Pager.journalOff, then write a 0x00
+        ** byte to the start of it to prevent it from being recognized.
+        */
+        rc = sqlite3OsRead(pPager->jfd, zMagic, 8, jrnlOff);
+        if( rc==SQLITE_OK && 0==memcmp(zMagic, aJournalMagic, 8) ){
+          static const u8 zerobyte = 0;
+          rc = sqlite3OsWrite(pPager->jfd, &zerobyte, 1, jrnlOff);
+        }
+        if( rc!=SQLITE_OK && rc!=SQLITE_IOERR_SHORT_READ ){
+          return rc;
+        }
+
         /* Write the nRec value into the journal file header. If in
         ** full-synchronous mode, sync the journal first. This ensures that
         ** all data has really hit the disk before nRec is updated to mark
@@ -2498,7 +2530,6 @@ static int syncJournal(Pager *pPager){
         ** is populated with 0xFFFFFFFF when the journal header is written
         ** and never needs to be updated.
         */
-        i64 jrnlOff;
         if( pPager->fullSync && 0==(iDc&SQLITE_IOCAP_SEQUENTIAL) ){
           PAGERTRACE(("SYNC journal of %d\n", PAGERID(pPager)));
           IOTRACE(("JSYNC %p\n", pPager))
@@ -2879,8 +2910,10 @@ static int pagerSharedLock(Pager *pPager){
       pPager->journalHdr = 0;
  
       /* Playback and delete the journal.  Drop the database write
-      ** lock and reacquire the read lock.
+      ** lock and reacquire the read lock. Purge the cache before
+      ** playing back the hot-journal so that we don't end up with
       */
+      sqlite3PcacheClear(pPager->pPCache);
       rc = pager_playback(pPager, 1);
       if( rc!=SQLITE_OK ){
         rc = pager_error(pPager, rc);

test/crash8.test

@@ -0,0 +1,112 @@
+# 2009 January 8
+#
+# The author disclaims copyright to this source code.  In place of
+# a legal notice, here is a blessing:
+#
+#    May you do good and not evil.
+#    May you find forgiveness for yourself and forgive others.
+#    May you share freely, never taking more than you give.
+#
+#***********************************************************************
+#
+# This test verifies a couple of specific potential data corruption 
+# scenarios involving crashes or power failures.
+#
+# $Id: crash8.test,v 1.1 2009/01/08 17:50:46 danielk1977 Exp $
+
+
+set testdir [file dirname $argv0]
+source $testdir/tester.tcl
+
+ifcapable !crashtest {
+  finish_test
+  return
+}
+
+do_test crash8-1.1 {
+  execsql {
+    CREATE TABLE t1(a, b);
+    CREATE INDEX i1 ON t1(a, b);
+    INSERT INTO t1 VALUES(1, randstr(1000,1000));
+    INSERT INTO t1 VALUES(2, randstr(1000,1000));
+    INSERT INTO t1 VALUES(3, randstr(1000,1000));
+    INSERT INTO t1 VALUES(4, randstr(1000,1000));
+    INSERT INTO t1 VALUES(5, randstr(1000,1000));
+    INSERT INTO t1 VALUES(6, randstr(1000,1000));
+    CREATE TABLE t2(a, b);
+    CREATE TABLE t3(a, b);
+    CREATE TABLE t4(a, b);
+    CREATE TABLE t5(a, b);
+    CREATE TABLE t6(a, b);
+    CREATE TABLE t7(a, b);
+    CREATE TABLE t8(a, b);
+    CREATE TABLE t9(a, b);
+    CREATE TABLE t10(a, b);
+    PRAGMA integrity_check
+  }
+} {ok}
+
+
+# Potential corruption scenario 1. A second process opens the database 
+# and modifies a large portion of it. It then opens a second transaction
+# and modifies a small part of the database, but crashes before it commits
+# the transaction. 
+#
+# When the first process accessed the database again, it was rolling back
+# the aborted transaction, but was not purging its in-memory cache (which
+# was loaded before the second process made its first, successful, 
+# modification). Producing an inconsistent cache.
+#
+do_test crash8-1.2 {
+  crashsql -delay 2 -file test.db {
+    PRAGMA cache_size = 10;
+    UPDATE t1 SET b = randstr(1000,1000);
+    INSERT INTO t9 VALUES(1, 2);
+  }
+} {1 {child process exited abnormally}}
+do_test crash8-1.3 {
+  execsql {PRAGMA integrity_check}
+} {ok}
+
+# Potential corruption scenario 2. The second process, operating in
+# persistent-journal mode, makes a large change to the database file
+# with a small in-memory cache. Such that more than one journal-header
+# was written to the file. It then opens a second transaction and makes
+# a smaller change that requires only a single journal-header to be
+# written to the journal file. The second change is such that the 
+# journal content written to the persistent journal file exactly overwrites
+# the first journal-header and set of subsequent records written by the
+# first, successful, change. The second process crashes before it can
+# commit its second change.
+#
+# When the first process accessed the database again, it was rolling back
+# the second aborted transaction, then continuing to rollback the second
+# and subsequent journal-headers written by the first, successful, change.
+# Database corruption.
+#
+do_test crash8.2.1 {
+  crashsql -delay 2 -file test.db {
+    PRAGMA journal_mode = persist;
+    PRAGMA cache_size = 10;
+    UPDATE t1 SET b = randstr(1000,1000);
+    PRAGMA cache_size = 100;
+    BEGIN;
+      INSERT INTO t2 VALUES('a', 'b');
+      INSERT INTO t3 VALUES('a', 'b');
+      INSERT INTO t4 VALUES('a', 'b');
+      INSERT INTO t5 VALUES('a', 'b');
+      INSERT INTO t6 VALUES('a', 'b');
+      INSERT INTO t7 VALUES('a', 'b');
+      INSERT INTO t8 VALUES('a', 'b');
+      INSERT INTO t9 VALUES('a', 'b');
+      INSERT INTO t10 VALUES('a', 'b');
+    COMMIT;
+  }
+} {1 {child process exited abnormally}}
+
+do_test crash8-2.3 {
+  execsql {PRAGMA integrity_check}
+} {ok}
+
+finish_test
+