From e965ac77737585dce3b58e1e318856020920bf5e Mon Sep 17 00:00:00 2001 From: danielk1977 Date: Wed, 13 Jun 2007 15:22:28 +0000 Subject: [PATCH] Fix for #2409. Return SQLITE_IOERR_BLOCKED instead of SQLITE_BUSY in cases where failure to obtain a database lock leaves the cache in an inconsistent state. See additional information at CorruptionFollowingBusyError. (CVS 4060) FossilOrigin-Name: ce2c9925d06315d73fb5fd0c7265fb4cd65665aa --- manifest | 19 +++-- manifest.uuid | 2 +- src/pager.c | 13 ++- src/sqlite.h.in | 3 +- src/vdbeaux.c | 5 +- test/tkt2409.test | 213 ++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 242 insertions(+), 13 deletions(-) create mode 100644 test/tkt2409.test diff --git a/manifest b/manifest index d67b073617..d000eb8561 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C In\sthe\s"transaction"\scommand\sof\sthe\sTCL\sinterface,\sif\sa\sCOMMIT\sfails\sfinish\nit\swith\sa\srollback.\s(CVS\s4059) -D 2007-06-12T18:50:14 +C Fix\sfor\s#2409.\sReturn\sSQLITE_IOERR_BLOCKED\sinstead\sof\sSQLITE_BUSY\sin\scases\swhere\sfailure\sto\sobtain\sa\sdatabase\slock\sleaves\sthe\scache\sin\san\sinconsistent\sstate.\r\nSee\sadditional\sinformation\sat\sCorruptionFollowingBusyError.\s(CVS\s4060) +D 2007-06-13T15:22:28 F Makefile.in 31d9f7cd42c3d73ae117fcdb4b0ecd029fa8f50b F Makefile.linux-gcc 2d8574d1ba75f129aba2019f0b959db380a90935 F README 9c4e2d6706bdcc3efdd773ce752a8cdab4f90028 @@ -94,7 +94,7 @@ F src/os_unix.c f2ccf2e9a925fc679faf7a8fe85700e0f13cf0e1 F src/os_unix.h 5768d56d28240d3fe4537fac08cc85e4fb52279e F src/os_win.c d868d5f9e95ec9c1b9e2a30c54c996053db6dddd F src/os_win.h 41a946bea10f61c158ce8645e7646b29d44f122b -F src/pager.c a193f8f0783e33ea0aa1965e951c3daa2b2c9c4e +F src/pager.c 42cffb2e04a6efd23cfff28a15d686fbaa076b59 F src/pager.h 94110a5570dca30d54a883e880a3633b2e4c05ae F src/parse.y e3d8e3f748bd3915523f77f704f303c7f0de613b F src/pragma.c 0d25dad58bdfd6789943a10f1b9663c2eb85b96d @@ -104,7 +104,7 @@ F src/random.c 6119474a6f6917f708c1dee25b9a8e519a620e88 F src/select.c a96d80c0493bf81f90415479b0055dc91f60a812 F src/server.c 087b92a39d883e3fa113cae259d64e4c7438bc96 F src/shell.c d07ae326b3815d80f71c69b3c7584382e47f6447 -F src/sqlite.h.in b174b5508467deec4034c6c8a21f0354b498b46b +F src/sqlite.h.in 724358b899d028ff44bfac7d0c071b49fcc6c0b7 F src/sqlite3ext.h 7d0d363ea7327e817ef0dfe1b7eee1f171b72890 F src/sqliteInt.h 208c40b6e11925a321ec159d889e0ec06b618359 F src/table.c a8de75bcedf84d4060d804264b067ab3b1a3561d @@ -138,7 +138,7 @@ F src/vdbe.c 265d7061e91ebd81ebf8337ce1bac52c0548522e F src/vdbe.h 001c5b257567c1d3de7feb2203aac71d0d7b16a3 F src/vdbeInt.h 7d2bf163d6d4e815724a457f2216dd8e38c3955c F src/vdbeapi.c 3747e4c3bc3139ff688bb3df462b10e42c084d16 -F src/vdbeaux.c a978d170b2ca99c8ff3da8a91f116a66da2600ac +F src/vdbeaux.c 85b374b68ba4c1140269c2659122bb1b44a1995c F src/vdbeblob.c 96f3572fdc45eda5be06e6372b612bc30742d9f0 F src/vdbefifo.c 3ca8049c561d5d67cbcb94dc909ae9bb68c0bf8f F src/vdbemem.c d86c25bbfe8102499ff7505fca44a779c68694d8 @@ -389,6 +389,7 @@ F test/tkt2285.test c618085f0c13ec3347e607f83c34ada0721b4bfa F test/tkt2332.test cb1bb0ed1ae6a3b9ff412520ed4a496745f4ffa5 F test/tkt2339.test 7016415bda86af1406a27260ac46f44885617606 F test/tkt2391.test ab7a11be7402da8b51a5be603425367aa0684567 +F test/tkt2409.test e0bc9d0534537b0027cfa9f43da31ff9829899cb F test/trace.test 75ffc1b992c780d054748a656e3e7fd674f18567 F test/trans.test 3fe1b9e03b523482eee2b869858c5c1eca7b218b F test/trigger1.test b361161cf20614024cc1e52ea0bdec250776b2ae @@ -503,7 +504,7 @@ F www/tclsqlite.tcl bb0d1357328a42b1993d78573e587c6dcbc964b9 F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0 F www/version3.tcl 890248cf7b70e60c383b0e84d77d5132b3ead42b F www/whentouse.tcl fc46eae081251c3c181bd79c5faef8195d7991a5 -P 6953cd0935b5526756ab745545420e40adc3c56d -R 5d27d3b997120cea0304bcfe42f3f02a -U drh -Z a17081a7409ebb25284cf9a87a711cc7 +P 6da39fa4429400e21924074f5f219f4cb32415ff +R 1bb49dc38a5d5f3caa65dd4b9eb8ab34 +U danielk1977 +Z a6c128ab59512ffa27dc80b804f1fa2e diff --git a/manifest.uuid b/manifest.uuid index d1d03af57a..e8db816d60 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -6da39fa4429400e21924074f5f219f4cb32415ff \ No newline at end of file +ce2c9925d06315d73fb5fd0c7265fb4cd65665aa \ No newline at end of file diff --git a/src/pager.c b/src/pager.c index d4a07f7bf4..7ff67bbb8c 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.342 2007/05/24 09:41:21 danielk1977 Exp $ +** @(#) $Id: pager.c,v 1.343 2007/06/13 15:22:28 danielk1977 Exp $ */ #ifndef SQLITE_OMIT_DISKIO #include "sqliteInt.h" @@ -2941,6 +2941,9 @@ static int pagerAllocatePage(Pager *pPager, PgHdr **ppPg){ }else{ /* Recycle an existing page with a zero ref-count. */ rc = pager_recycle(pPager, 1, &pPg); + if( rc==SQLITE_BUSY ){ + rc = SQLITE_IOERR_BLOCKED; + } if( rc!=SQLITE_OK ){ goto pager_allocate_out; } @@ -3886,6 +3889,14 @@ int sqlite3PagerCommitPhaseOne(Pager *pPager, const char *zMaster, Pgno nTrunc){ } sync_exit: + if( rc==SQLITE_IOERR_BLOCKED ){ + /* pager_incr_changecounter() may attempt to obtain an exclusive + * lock to spill the cache and return IOERR_BLOCKED. But since + * there is no chance the cache is inconsistent, it's + * better to return SQLITE_BUSY. + */ + rc = SQLITE_BUSY; + } return rc; } diff --git a/src/sqlite.h.in b/src/sqlite.h.in index f06f107c44..fa6830bd90 100644 --- a/src/sqlite.h.in +++ b/src/sqlite.h.in @@ -12,7 +12,7 @@ ** This header file defines the interface that the SQLite library ** presents to client programs. ** -** @(#) $Id: sqlite.h.in,v 1.210 2007/05/24 09:44:11 danielk1977 Exp $ +** @(#) $Id: sqlite.h.in,v 1.211 2007/06/13 15:22:28 danielk1977 Exp $ */ #ifndef _SQLITE3_H_ #define _SQLITE3_H_ @@ -231,6 +231,7 @@ int sqlite3_exec( #define SQLITE_IOERR_UNLOCK (SQLITE_IOERR | (8<<8)) #define SQLITE_IOERR_RDLOCK (SQLITE_IOERR | (9<<8)) #define SQLITE_IOERR_DELETE (SQLITE_IOERR | (10<<8)) +#define SQLITE_IOERR_BLOCKED (SQLITE_IOERR | (11<<8)) /* ** Enable or disable the extended result codes. diff --git a/src/vdbeaux.c b/src/vdbeaux.c index c62b928fb7..47bb7a081f 100644 --- a/src/vdbeaux.c +++ b/src/vdbeaux.c @@ -1391,7 +1391,10 @@ int sqlite3VdbeHalt(Vdbe *p){ ** proceed with the special handling. */ if( !isReadOnly ){ - if( p->rc==SQLITE_NOMEM && isStatement ){ + if( p->rc==SQLITE_IOERR_BLOCKED && isStatement ){ + xFunc = sqlite3BtreeRollbackStmt; + p->rc = SQLITE_BUSY; + } else if( p->rc==SQLITE_NOMEM && isStatement ){ xFunc = sqlite3BtreeRollbackStmt; }else{ /* We are forced to roll back the active transaction. Before doing diff --git a/test/tkt2409.test b/test/tkt2409.test new file mode 100644 index 0000000000..3e36a275b6 --- /dev/null +++ b/test/tkt2409.test @@ -0,0 +1,213 @@ +# 2007 June 13 +# +# 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 file implements regression tests for SQLite library. +# +# This file implements tests to verify that ticket #2409 has been +# fixed. More specifically, they verify that if SQLite cannot +# obtain an EXCLUSIVE lock while trying to spill the cache during +# any statement other than a COMMIT, an I/O error is returned instead +# of SQLITE_BUSY. +# +# $Id: tkt2409.test,v 1.1 2007/06/13 15:22:28 danielk1977 Exp $ + +# Test Outline: +# +# tkt-2409-1.*: Cause a cache-spill during an INSERT that is within +# a db transaction but does not start a statement transaction. +# Verify that the transaction is automatically rolled back +# and SQLITE_IOERR_BLOCKED is returned +# +# tkt-2409-2.*: Cause a cache-spill while updating the change-counter +# during a database COMMIT. Verify that the transaction is not +# rolled back and SQLITE_BUSY is returned. +# +# tkt-2409-3.*: Similar to 2409-1.*, but using many INSERT statements +# within a transaction instead of just one. +# +# tkt-2409-4.*: Similar to 2409-1.*, but rig it so that the +# INSERT statement starts a statement transaction. Verify that +# SQLOTE_BUSY is returned and the transaction is not rolled back. +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl + +sqlite3_extended_result_codes $::DB 1 + +# Aquire a read-lock on the database using handle [db2]. +# +proc read_lock_db {} { + if {$::STMT eq ""} { + set ::STMT [sqlite3_prepare db2 {SELECT rowid FROM sqlite_master} -1 TAIL] + set rc [sqlite3_step $::STMT] + if {$rc eq "SQLITE_ERROR"} { + unread_lock_db + read_lock_db + } + } +} + +# Release any read-lock obtained using [read_lock_db] +# +proc unread_lock_db {} { + if {$::STMT ne ""} { + sqlite3_finalize $::STMT + set ::STMT "" + } +} + +# Open the db handle used by [read_lock_db]. +# +sqlite3 db2 test.db +set ::STMT "" + +do_test tkt2409-1.1 { + execsql { + PRAGMA cache_size=10; + CREATE TABLE t1(x TEXT UNIQUE NOT NULL, y BLOB); + } + read_lock_db + set ::zShort [string repeat 0123456789 1] + set ::zLong [string repeat 0123456789 1500] + catchsql { + BEGIN; + INSERT INTO t1 VALUES($::zShort, $::zLong); + } +} {1 {disk I/O error}} + +do_test tkt2409-1.2 { + sqlite3_errcode $::DB +} {SQLITE_IOERR+11} + +# Check the integrity of the cache. +# +integrity_check tkt2409-1.3 + +# Check that the transaction was rolled back. Because the INSERT +# statement in which the "I/O error" occured did not open a statement +# transaction, SQLite had no choice but to roll back the transaction. +# +do_test tkt2409-1.4 { + unread_lock_db + catchsql { ROLLBACK } +} {1 {cannot rollback - no transaction is active}} + + +set ::zShort [string repeat 0123456789 1] +set ::zLong [string repeat 0123456789 1500] +set ::rc 1 +for {set iCache 10} {$::rc} {incr iCache} { + execsql "PRAGMA cache_size = $iCache" + do_test tkt2409-2.1.$iCache { + read_lock_db + set ::rc [catch { + execsql { + BEGIN; + INSERT INTO t1 VALUES($::zShort, $::zLong); + } + } msg] + expr {($::rc == 1 && $msg eq "disk I/O error") || $::rc == 0} + } {1} +} + +do_test tkt2409-2.2 { + catchsql { + ROLLBACK; + BEGIN; + INSERT INTO t1 VALUES($::zShort, $::zLong); + COMMIT; + } +} {1 {database is locked}} + +do_test tkt2409-2.3 { + unread_lock_db + catchsql { + COMMIT; + } +} {0 {}} + +do_test tkt2409-3.1 { + db close + set ::DB [sqlite3 db test.db; sqlite3_connection_pointer db] + sqlite3_extended_result_codes $::DB 1 + execsql { + PRAGMA cache_size=10; + DELETE FROM t1; + } + read_lock_db + set ::zShort [string repeat 0123456789 1] + set ::zLong [string repeat 0123456789 1500] + catchsql { + BEGIN; + INSERT INTO t1 SELECT $::zShort, $::zLong; + } +} {1 {database is locked}} + +do_test tkt2409-3.2 { + sqlite3_errcode $::DB +} {SQLITE_BUSY} + +# Check the integrity of the cache. +# +integrity_check tkt2409-3.3 + +# Check that the transaction was rolled back. Because the INSERT +# statement in which the "I/O error" occured did not open a statement +# transaction, SQLite had no choice but to roll back the transaction. +# +do_test tkt2409-3.4 { + unread_lock_db + catchsql { ROLLBACK } +} {0 {}} + + +do_test tkt2409-4.1 { + execsql { + PRAGMA cache_size=20; + DROP TABLE t1; + CREATE TABLE t1 (x TEXT UNIQUE NOT NULL); + } + + unset -nocomplain t1 + array unset t1 + set t1(0) 1 + set sql "" + for {set i 0} {$i<5000} {incr i} { + set r 0 + while {[info exists t1($r)]} { + set r [expr {int(rand()*1000000000)}] + } + set t1($r) 1 + append sql "INSERT INTO t1 VALUES('some-text-$r');" + } + + read_lock_db + execsql BEGIN + catchsql $sql +} {1 {disk I/O error}} + +do_test tkt2409-4.2 { + sqlite3_errcode $::DB +} {SQLITE_IOERR+11} + +# Check the integrity of the cache. +# +integrity_check tkt2409-4.3 + +do_test tkt2409-4.4 { + catchsql { ROLLBACK } +} {1 {cannot rollback - no transaction is active}} + + +unread_lock_db +db2 close +finish_test +