diff --git a/ext/session/session_common.tcl b/ext/session/session_common.tcl index c1a6440396..41c80dee82 100644 --- a/ext/session/session_common.tcl +++ b/ext/session/session_common.tcl @@ -42,9 +42,25 @@ proc do_common_sql {sql} { execsql $sql db2 } +proc changeset_from_sql {sql {dbname main}} { + set rc [catch { + sqlite3session S db $dbname + db eval "SELECT name FROM $dbname.sqlite_master WHERE type = 'table'" { + S attach $name + } + db eval $sql + S changeset + } changeset] + catch { S delete } + + if {$rc} { + error $changeset + } + return $changeset +} + proc do_then_apply_sql {sql {dbname main}} { proc xConflict args { return "OMIT" } - set rc [catch { sqlite3session S db $dbname db eval "SELECT name FROM $dbname.sqlite_master WHERE type = 'table'" { @@ -100,7 +116,11 @@ proc compare_db {db1 db2} { set sql "SELECT * FROM $tbl ORDER BY [join $col1 ,]" set data1 [$db1 eval $sql] set data2 [$db2 eval $sql] - if {$data1 != $data2} { error "table $tbl data mismatch" } + if {$data1 != $data2} { + puts "$data1" + puts "$data2" + error "table $tbl data mismatch" + } } return "" diff --git a/ext/session/sessionfault.test b/ext/session/sessionfault.test index 8f38fe63e8..7ba05bbb22 100644 --- a/ext/session/sessionfault.test +++ b/ext/session/sessionfault.test @@ -22,7 +22,6 @@ set testprefix sessionfault forcedelete test.db2 sqlite3 db2 test.db2 - do_common_sql { CREATE TABLE t1(a, b, c, PRIMARY KEY(a, b)); INSERT INTO t1 VALUES(1, 2, 3); @@ -31,6 +30,8 @@ do_common_sql { faultsim_save_and_close db2 close + +#------------------------------------------------------------------------- # Test OOM error handling when collecting and applying a simple changeset. # do_faultsim_test pagerfault-1 -faults oom-* -prep { @@ -50,11 +51,58 @@ do_faultsim_test pagerfault-1 -faults oom-* -prep { if {$testrc==0} { compare_db db db2 } } +#------------------------------------------------------------------------- +# The following block of tests - pagerfault-2.* - are designed to check +# the handling of faults in the sqlite3changeset_apply() function. +# +catch {db close} +catch {db2 close} +forcedelete test.db2 test.db +sqlite3 db2 test.db2 +sqlite3 db test.db +do_common_sql { + CREATE TABLE t1(a, b, c, PRIMARY KEY(a, b)); + INSERT INTO t1 VALUES('apple', 'orange', 'pear'); + + CREATE TABLE t2(x PRIMARY KEY, y); +} +db2 close +faultsim_save_and_close + + +foreach {tn conflict_policy sql sql2} { + 1 OMIT { INSERT INTO t1 VALUES('one text', 'two text', X'00ff00') } {} + 2 OMIT { DELETE FROM t1 WHERE a = 'apple' } {} + 3 OMIT { UPDATE t1 SET c = 'banana' WHERE b = 'orange' } {} + 4 REPLACE { INSERT INTO t2 VALUES('keyvalue', 'value 1') } { + INSERT INTO t2 VALUES('keyvalue', 'value 2'); + } +} { + proc xConflict args [list return $conflict_policy] + + do_faultsim_test pagerfault-2.$tn -faults oom-transient -prep { + catch {db2 close} + catch {db close} + faultsim_restore_and_reopen + set ::changeset [changeset_from_sql $::sql] + sqlite3 db2 test.db2 + sqlite3_db_config_lookaside db2 0 0 0 + execsql $::sql2 db2 + } -body { + sqlite3changeset_apply db2 $::changeset xConflict + } -test { + faultsim_test_result {0 {}} {1 SQLITE_NOMEM} + faultsim_integrity_check + if {$testrc==0} { compare_db db db2 } + } +} + +#------------------------------------------------------------------------- # This test case is designed so that a malloc() failure occurs while # resizing the session object hash-table from 256 to 512 buckets. This # is not an error, just a sub-optimal condition. # -do_faultsim_test pagerfault-2 -faults oom-* -prep { +do_faultsim_test pagerfault-3 -faults oom-* -prep { catch {db2 close} catch {db close} faultsim_restore_and_reopen @@ -95,7 +143,7 @@ proc xConflict {op tbl type args} { return "OMIT" } -do_test 3.0 { +do_test 4.0 { execsql { PRAGMA encoding = 'utf16'; CREATE TABLE t1(a PRIMARY KEY, b); @@ -113,7 +161,7 @@ do_test 3.0 { faultsim_save_and_close db2 close -do_faultsim_test pagerfault-3 -faults oom-transient -prep { +do_faultsim_test pagerfault-4 -faults oom-* -prep { catch {db2 close} catch {db close} faultsim_restore_and_reopen @@ -134,4 +182,43 @@ do_faultsim_test pagerfault-3 -faults oom-transient -prep { if {$testrc==0} { compare_db db db2 } } +#------------------------------------------------------------------------- +# This block of tests verifies that OOM faults in the +# sqlite3changeset_invert() function are handled correctly. +# +catch {db close} +catch {db2 close} +forcedelete test.db +sqlite3 db test.db +execsql { + CREATE TABLE t1(a, b, PRIMARY KEY(b)); + CREATE TABLE t2(a PRIMARY KEY, b); + INSERT INTO t1 VALUES('string', 1); + INSERT INTO t1 VALUES(4, 2); + INSERT INTO t1 VALUES(X'FFAAFFAAFFAA', 3); +} +set changeset [changeset_from_sql { + INSERT INTO t1 VALUES('xxx', 'yyy'); + DELETE FROM t1 WHERE a = 'string'; + UPDATE t1 SET a = 20 WHERE b = 2; +}] +db close + +do_faultsim_test pagerfault-5 -faults oom* -body { + set ::inverse [sqlite3changeset_invert $::changeset] + set {} {} +} -test { + faultsim_test_result {0 {}} {1 SQLITE_NOMEM} + if {$testrc==0} { + set x [list] + sqlite3session_foreach c $::inverse { lappend x $c } + foreach c { + {DELETE t1 {t xxx t yyy} {}} + {INSERT t1 {} {t string i 1}} + {UPDATE t1 {i 20 {} {}} {i 4 i 2}} + } { lappend y $c } + if {$x != $y} { error "changeset no good" } + } +} + finish_test diff --git a/ext/session/sqlite3session.c b/ext/session/sqlite3session.c index 3c6a972961..7c7890aed2 100644 --- a/ext/session/sqlite3session.c +++ b/ext/session/sqlite3session.c @@ -1637,6 +1637,14 @@ int sqlite3changeset_new( return SQLITE_OK; } +/* +** The following two macros are used internally. They are similar to the +** sqlite3changeset_new() and sqlite3changeset_old() functions, except that +** they omit all error checking and return a pointer to the requested value. +*/ +#define sessionChangesetNew(pIter, iVal) (pIter)->apValue[(pIter)->nCol+(iVal)] +#define sessionChangesetOld(pIter, iVal) (pIter)->apValue[(iVal)] + /* ** This function may only be called with a changeset iterator that has been ** passed to an SQLITE_CHANGESET_DATA or SQLITE_CHANGESET_CONFLICT @@ -1989,6 +1997,24 @@ static int sessionInsertRow( return rc; } +/* +** A wrapper around sqlite3_bind_value() that detects an extra problem. +** See comments in the body of this function for details. +*/ +static int sessionBindValue( + sqlite3_stmt *pStmt, /* Statement to bind value to */ + int i, /* Parameter number to bind to */ + sqlite3_value *pVal /* Value to bind */ +){ + if( (pVal->type==SQLITE_TEXT || pVal->type==SQLITE_BLOB) && pVal->z==0 ){ + /* This condition occurs when an earlier OOM in a call to + ** sqlite3_value_text() or sqlite3_value_blob() (perhaps from within + ** a conflict-hanler) has zeroed the pVal->z pointer. Return NOMEM. */ + return SQLITE_NOMEM; + } + return sqlite3_bind_value(pStmt, i, pVal); +} + /* ** Iterator pIter must point to an SQLITE_INSERT entry. This function ** transfers new.* values from the current iterator entry to statement @@ -2003,22 +2029,27 @@ static int sessionInsertRow( ** ** An SQLite error code is returned if an error occurs. Otherwise, SQLITE_OK. */ -static int sessionBindValues( +static int sessionBindRow( sqlite3_changeset_iter *pIter, /* Iterator to read values from */ - int(*xIterValue)(sqlite3_changeset_iter *, int, sqlite3_value **), + int(*xValue)(sqlite3_changeset_iter *, int, sqlite3_value **), int nCol, /* Number of columns */ u8 *abPK, /* If not NULL, bind only if true */ sqlite3_stmt *pStmt /* Bind values to this statement */ ){ int i; int rc = SQLITE_OK; + + /* Neither sqlite3changeset_old or sqlite3changeset_new can fail if the + ** argument iterator points to a suitable entry. Make sure that xValue + ** is one of these to guarantee that it is safe to ignore the return + ** in the code below. */ + assert( xValue==sqlite3changeset_old || xValue==sqlite3changeset_new ); + for(i=0; rc==SQLITE_OK && ipSelect); }else if( rc==SQLITE_OK ){ /* No other row with the new.* primary key. */ - rc = sqlite3_reset(p->pSelect); - if( rc==SQLITE_OK ){ - res = xConflict(pCtx, eType+1, pIter); - if( res==SQLITE_CHANGESET_REPLACE ) rc = SQLITE_MISUSE; - } + res = xConflict(pCtx, eType+1, pIter); + if( res==SQLITE_CHANGESET_REPLACE ) rc = SQLITE_MISUSE; } if( rc==SQLITE_OK ){ @@ -2207,7 +2239,7 @@ static int sessionApplyOneOp( if( op==SQLITE_DELETE ){ /* Bind values to the DELETE statement. */ - rc = sessionBindValues(pIter, sqlite3changeset_old, nCol, 0, p->pDelete); + rc = sessionBindRow(pIter, sqlite3changeset_old, nCol, 0, p->pDelete); if( rc==SQLITE_OK && sqlite3_bind_parameter_count(p->pDelete)>nCol ){ rc = sqlite3_bind_int(p->pDelete, nCol+1, pbRetry==0); } @@ -2230,19 +2262,18 @@ static int sessionApplyOneOp( /* Bind values to the UPDATE statement. */ for(i=0; rc==SQLITE_OK && ipUpdate, i*3+2, !!pNew); + if( pOld ){ + rc = sessionBindValue(p->pUpdate, i*3+1, pOld); } - if( rc==SQLITE_OK ){ - if( pOld ) sqlite3_bind_value(p->pUpdate, i*3+1, pOld); - sqlite3_bind_int(p->pUpdate, i*3+2, !!pNew); - if( pNew ) sqlite3_bind_value(p->pUpdate, i*3+3, pNew); + if( rc==SQLITE_OK && pNew ){ + rc = sessionBindValue(p->pUpdate, i*3+3, pNew); } } - if( rc==SQLITE_OK ) rc = sqlite3_bind_int(p->pUpdate, nCol*3+1, pbRetry==0); + if( rc==SQLITE_OK ) sqlite3_bind_int(p->pUpdate, nCol*3+1, pbRetry==0); if( rc!=SQLITE_OK ) return rc; /* Attempt the UPDATE. In the case of a NOTFOUND or DATA conflict, @@ -2268,7 +2299,7 @@ static int sessionApplyOneOp( }else{ assert( op==SQLITE_INSERT ); - rc = sessionBindValues(pIter, sqlite3changeset_new, nCol, 0, p->pInsert); + rc = sessionBindRow(pIter, sqlite3changeset_new, nCol, 0, p->pInsert); if( rc!=SQLITE_OK ) return rc; sqlite3_step(p->pInsert); @@ -2354,7 +2385,7 @@ int sqlite3changeset_apply( assert( pIter->op==SQLITE_INSERT ); rc = sqlite3_exec(db, "SAVEPOINT replace_op", 0, 0, 0); if( rc==SQLITE_OK ){ - rc = sessionBindValues(pIter, + rc = sessionBindRow(pIter, sqlite3changeset_new, sApply.nCol, sApply.abPK, sApply.pDelete); sqlite3_bind_int(sApply.pDelete, sApply.nCol+1, 1); } diff --git a/manifest b/manifest index 6310e57e5f..a2a6acaf3d 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\sa\scouple\stypos\sfor\sconsistency\sin\ssessions\sdocumentation. -D 2011-03-22T02:03:23.754 +C Add\sOOM\stests\sand\srelated\sfixes\sfor\sthe\ssession\smodule. +D 2011-03-22T12:08:00 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in 27701a1653595a1f2187dc61c8117e00a6c1d50f F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -101,9 +101,9 @@ F ext/rtree/tkt3363.test 142ab96eded44a3615ec79fba98c7bde7d0f96de F ext/rtree/viewrtree.tcl eea6224b3553599ae665b239bd827e182b466024 F ext/session/session1.test 1e8cda2cc8a60171dabc0fbec4124f9f7c943f23 F ext/session/session2.test 54c3a5ecdc60548a8ab0a86793b136de6f32a255 -F ext/session/session_common.tcl d7bb85c3fd76d53bd9b909da808d5c16f5213111 -F ext/session/sessionfault.test da234166d5d044c91964863174d7171b0561708b -F ext/session/sqlite3session.c 1ca39db8a10b8bcb973b35a68c0924b6a64c4a97 +F ext/session/session_common.tcl c40e81e86b46adc92b7bcb0182eaa70c8523ad8e +F ext/session/sessionfault.test 2dcf303379d0c01d8320f3c7d0452e6a0dcebd48 +F ext/session/sqlite3session.c 2fd432583cc79928b6d31792fc4cb8bfc3e50588 F ext/session/sqlite3session.h 2b7b2b983bba8f7a299922056884912084b32c68 F ext/session/test_session.c 2559ef68e421c7fb83e2c19ef08a17343b70d535 F install-sh 9d4de14ab9fb0facae2f48780b874848cbf2f895 @@ -923,7 +923,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f -P 39cdfa5324ae91bfbbac733b1e3e2d33ca883340 -R 7e8df5e06663856423ec54de25f0cd9f -U shaneh -Z a8ae98277851bb377ac869793dff328e +P 510198f171b9f77a4ad49c06c978c5fbb3a5b7f9 +R 64f2c206ae40e85e14ca00143e665617 +U dan +Z 4313381608751ccada39ab0745df4a8f diff --git a/manifest.uuid b/manifest.uuid index e429e99a7c..58da30c0cd 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -510198f171b9f77a4ad49c06c978c5fbb3a5b7f9 \ No newline at end of file +06048a68b351e3eb15a890cb54db8a1d4b345fbc \ No newline at end of file