/ Check-in [5a39525b]
Login

Many hyperlinks are disabled.
Use anonymous login to enable hyperlinks.

Overview
Comment:Fix a couple of potential corruption problems in pager.c. (CVS 6143)
Downloads: Tarball | ZIP archive | SQL archive
Timelines: family | ancestors | descendants | both | trunk
Files: files | file ages | folders
SHA1:5a39525ba3e65f1c6df3cf23fbfb57f6a0bf4b62
User & Date: danielk1977 2009-01-08 17:50:46
Context
2009-01-08
17:57
Avoid an 'invalid cast' warning in test_osinst.c. (CVS 6144) check-in: 931f3a21 user: danielk1977 tags: trunk
17:50
Fix a couple of potential corruption problems in pager.c. (CVS 6143) check-in: 5a39525b user: danielk1977 tags: trunk
15:24
Add a test script for ticket #2565. Change the assert() in pager.c into a testcase() macro. (CVS 6142) check-in: 1e53e382 user: drh tags: trunk
Changes
Hide Diffs Side-by-Side Diffs Ignore Whitespace Patch

Changes to src/pager.c.

    14     14   ** The pager is used to access a database disk file.  It implements
    15     15   ** atomic commit and rollback through the use of a journal file that
    16     16   ** is separate from the database file.  The pager also implements file
    17     17   ** locking to prevent two processes from writing the same database
    18     18   ** file simultaneously, or one process from reading the database while
    19     19   ** another is writing.
    20     20   **
    21         -** @(#) $Id: pager.c,v 1.541 2009/01/08 15:24:02 drh Exp $
           21  +** @(#) $Id: pager.c,v 1.542 2009/01/08 17:50:46 danielk1977 Exp $
    22     22   */
    23     23   #ifndef SQLITE_OMIT_DISKIO
    24     24   #include "sqliteInt.h"
    25     25   
    26     26   /*
    27     27   ** Macros for troubleshooting.  Normally turned off
    28     28   */
................................................................................
   576    576   ** ---------------------------------------
   577    577   ** 0                         0
   578    578   ** 512                       512
   579    579   ** 100                       512
   580    580   ** 2000                      2048
   581    581   ** 
   582    582   */
   583         -static void seekJournalHdr(Pager *pPager){
          583  +static i64 journalHdrOffset(Pager *pPager){
   584    584     i64 offset = 0;
   585    585     i64 c = pPager->journalOff;
   586    586     if( c ){
   587    587       offset = ((c-1)/JOURNAL_HDR_SZ(pPager) + 1) * JOURNAL_HDR_SZ(pPager);
   588    588     }
   589    589     assert( offset%JOURNAL_HDR_SZ(pPager)==0 );
   590    590     assert( offset>=c );
   591    591     assert( (offset-c)<JOURNAL_HDR_SZ(pPager) );
   592         -  pPager->journalOff = offset;
          592  +  return offset;
          593  +}
          594  +static void seekJournalHdr(Pager *pPager){
          595  +  pPager->journalOff = journalHdrOffset(pPager);
   593    596   }
   594    597   
   595    598   /*
   596    599   ** Write zeros over the header of the journal file.  This has the
   597    600   ** effect of invalidating the journal file and committing the
   598    601   ** transaction.
   599    602   */
................................................................................
  2483   2486     if( pPager->needSync ){
  2484   2487       assert( !pPager->tempFile );
  2485   2488       if( pPager->journalMode!=PAGER_JOURNALMODE_MEMORY ){
  2486   2489         int iDc = sqlite3OsDeviceCharacteristics(pPager->fd);
  2487   2490         assert( pPager->journalOpen );
  2488   2491   
  2489   2492         if( 0==(iDc&SQLITE_IOCAP_SAFE_APPEND) ){
         2493  +        i64 jrnlOff = journalHdrOffset(pPager);
         2494  +        u8 zMagic[8];
         2495  +
         2496  +        /* This block deals with an obscure problem. If the last connection
         2497  +        ** that wrote to this database was operating in persistent-journal
         2498  +        ** mode, then the journal file may at this point actually be larger
         2499  +        ** than Pager.journalOff bytes. If the next thing in the journal
         2500  +        ** file happens to be a journal-header (written as part of the
         2501  +        ** previous connections transaction), and a crash or power-failure 
         2502  +        ** occurs after nRec is updated but before this connection writes 
         2503  +        ** anything else to the journal file (or commits/rolls back its 
         2504  +        ** transaction), then SQLite may become confused when doing the 
         2505  +        ** hot-journal rollback following recovery. It may roll back all
         2506  +        ** of this connections data, then proceed to rolling back the old,
         2507  +        ** out-of-date data that follows it. Database corruption.
         2508  +        **
         2509  +        ** To work around this, if the journal file does appear to contain
         2510  +        ** a valid header following Pager.journalOff, then write a 0x00
         2511  +        ** byte to the start of it to prevent it from being recognized.
         2512  +        */
         2513  +        rc = sqlite3OsRead(pPager->jfd, zMagic, 8, jrnlOff);
         2514  +        if( rc==SQLITE_OK && 0==memcmp(zMagic, aJournalMagic, 8) ){
         2515  +          static const u8 zerobyte = 0;
         2516  +          rc = sqlite3OsWrite(pPager->jfd, &zerobyte, 1, jrnlOff);
         2517  +        }
         2518  +        if( rc!=SQLITE_OK && rc!=SQLITE_IOERR_SHORT_READ ){
         2519  +          return rc;
         2520  +        }
         2521  +
  2490   2522           /* Write the nRec value into the journal file header. If in
  2491   2523           ** full-synchronous mode, sync the journal first. This ensures that
  2492   2524           ** all data has really hit the disk before nRec is updated to mark
  2493   2525           ** it as a candidate for rollback.
  2494   2526           **
  2495   2527           ** This is not required if the persistent media supports the
  2496   2528           ** SAFE_APPEND property. Because in this case it is not possible 
  2497   2529           ** for garbage data to be appended to the file, the nRec field
  2498   2530           ** is populated with 0xFFFFFFFF when the journal header is written
  2499   2531           ** and never needs to be updated.
  2500   2532           */
  2501         -        i64 jrnlOff;
  2502   2533           if( pPager->fullSync && 0==(iDc&SQLITE_IOCAP_SEQUENTIAL) ){
  2503   2534             PAGERTRACE(("SYNC journal of %d\n", PAGERID(pPager)));
  2504   2535             IOTRACE(("JSYNC %p\n", pPager))
  2505   2536             rc = sqlite3OsSync(pPager->jfd, pPager->sync_flags);
  2506   2537             if( rc!=0 ) return rc;
  2507   2538           }
  2508   2539   
................................................................................
  2875   2906         pPager->journalOpen = 1;
  2876   2907         pPager->journalStarted = 0;
  2877   2908         pPager->journalOff = 0;
  2878   2909         pPager->setMaster = 0;
  2879   2910         pPager->journalHdr = 0;
  2880   2911    
  2881   2912         /* Playback and delete the journal.  Drop the database write
  2882         -      ** lock and reacquire the read lock.
         2913  +      ** lock and reacquire the read lock. Purge the cache before
         2914  +      ** playing back the hot-journal so that we don't end up with
  2883   2915         */
         2916  +      sqlite3PcacheClear(pPager->pPCache);
  2884   2917         rc = pager_playback(pPager, 1);
  2885   2918         if( rc!=SQLITE_OK ){
  2886   2919           rc = pager_error(pPager, rc);
  2887   2920           goto failed;
  2888   2921         }
  2889   2922         assert(pPager->state==PAGER_SHARED || 
  2890   2923             (pPager->exclusiveMode && pPager->state>PAGER_SHARED)

Added test/crash8.test.

            1  +# 2009 January 8
            2  +#
            3  +# The author disclaims copyright to this source code.  In place of
            4  +# a legal notice, here is a blessing:
            5  +#
            6  +#    May you do good and not evil.
            7  +#    May you find forgiveness for yourself and forgive others.
            8  +#    May you share freely, never taking more than you give.
            9  +#
           10  +#***********************************************************************
           11  +#
           12  +# This test verifies a couple of specific potential data corruption 
           13  +# scenarios involving crashes or power failures.
           14  +#
           15  +# $Id: crash8.test,v 1.1 2009/01/08 17:50:46 danielk1977 Exp $
           16  +
           17  +
           18  +set testdir [file dirname $argv0]
           19  +source $testdir/tester.tcl
           20  +
           21  +ifcapable !crashtest {
           22  +  finish_test
           23  +  return
           24  +}
           25  +
           26  +do_test crash8-1.1 {
           27  +  execsql {
           28  +    CREATE TABLE t1(a, b);
           29  +    CREATE INDEX i1 ON t1(a, b);
           30  +    INSERT INTO t1 VALUES(1, randstr(1000,1000));
           31  +    INSERT INTO t1 VALUES(2, randstr(1000,1000));
           32  +    INSERT INTO t1 VALUES(3, randstr(1000,1000));
           33  +    INSERT INTO t1 VALUES(4, randstr(1000,1000));
           34  +    INSERT INTO t1 VALUES(5, randstr(1000,1000));
           35  +    INSERT INTO t1 VALUES(6, randstr(1000,1000));
           36  +    CREATE TABLE t2(a, b);
           37  +    CREATE TABLE t3(a, b);
           38  +    CREATE TABLE t4(a, b);
           39  +    CREATE TABLE t5(a, b);
           40  +    CREATE TABLE t6(a, b);
           41  +    CREATE TABLE t7(a, b);
           42  +    CREATE TABLE t8(a, b);
           43  +    CREATE TABLE t9(a, b);
           44  +    CREATE TABLE t10(a, b);
           45  +    PRAGMA integrity_check
           46  +  }
           47  +} {ok}
           48  +
           49  +
           50  +# Potential corruption scenario 1. A second process opens the database 
           51  +# and modifies a large portion of it. It then opens a second transaction
           52  +# and modifies a small part of the database, but crashes before it commits
           53  +# the transaction. 
           54  +#
           55  +# When the first process accessed the database again, it was rolling back
           56  +# the aborted transaction, but was not purging its in-memory cache (which
           57  +# was loaded before the second process made its first, successful, 
           58  +# modification). Producing an inconsistent cache.
           59  +#
           60  +do_test crash8-1.2 {
           61  +  crashsql -delay 2 -file test.db {
           62  +    PRAGMA cache_size = 10;
           63  +    UPDATE t1 SET b = randstr(1000,1000);
           64  +    INSERT INTO t9 VALUES(1, 2);
           65  +  }
           66  +} {1 {child process exited abnormally}}
           67  +do_test crash8-1.3 {
           68  +  execsql {PRAGMA integrity_check}
           69  +} {ok}
           70  +
           71  +# Potential corruption scenario 2. The second process, operating in
           72  +# persistent-journal mode, makes a large change to the database file
           73  +# with a small in-memory cache. Such that more than one journal-header
           74  +# was written to the file. It then opens a second transaction and makes
           75  +# a smaller change that requires only a single journal-header to be
           76  +# written to the journal file. The second change is such that the 
           77  +# journal content written to the persistent journal file exactly overwrites
           78  +# the first journal-header and set of subsequent records written by the
           79  +# first, successful, change. The second process crashes before it can
           80  +# commit its second change.
           81  +#
           82  +# When the first process accessed the database again, it was rolling back
           83  +# the second aborted transaction, then continuing to rollback the second
           84  +# and subsequent journal-headers written by the first, successful, change.
           85  +# Database corruption.
           86  +#
           87  +do_test crash8.2.1 {
           88  +  crashsql -delay 2 -file test.db {
           89  +    PRAGMA journal_mode = persist;
           90  +    PRAGMA cache_size = 10;
           91  +    UPDATE t1 SET b = randstr(1000,1000);
           92  +    PRAGMA cache_size = 100;
           93  +    BEGIN;
           94  +      INSERT INTO t2 VALUES('a', 'b');
           95  +      INSERT INTO t3 VALUES('a', 'b');
           96  +      INSERT INTO t4 VALUES('a', 'b');
           97  +      INSERT INTO t5 VALUES('a', 'b');
           98  +      INSERT INTO t6 VALUES('a', 'b');
           99  +      INSERT INTO t7 VALUES('a', 'b');
          100  +      INSERT INTO t8 VALUES('a', 'b');
          101  +      INSERT INTO t9 VALUES('a', 'b');
          102  +      INSERT INTO t10 VALUES('a', 'b');
          103  +    COMMIT;
          104  +  }
          105  +} {1 {child process exited abnormally}}
          106  +
          107  +do_test crash8-2.3 {
          108  +  execsql {PRAGMA integrity_check}
          109  +} {ok}
          110  +
          111  +finish_test
          112  +