Add OOM tests and related fixes for the session module.

FossilOrigin-Name: 06048a68b351e3eb15a890cb54db8a1d4b345fbc

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 ""

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

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 && i<nCol; i++){
     if( !abPK || abPK[i] ){
       sqlite3_value *pVal;
-      rc = xIterValue(pIter, i, &pVal);
+      (void)xValue(pIter, i, &pVal);
-      if( rc==SQLITE_OK ){
+      rc = sessionBindValue(pStmt, i+1, pVal);
-        rc = sqlite3_bind_value(pStmt, i+1, pVal);
-      }
     }
   }
   return rc;
@@ -2031,7 +2062,11 @@ static int sessionBindValues(
 ** entry. If a row is found, the SELECT statement left pointing at the row 
 ** and SQLITE_ROW is returned. Otherwise, if no row is found and no error
 ** has occured, the statement is reset and SQLITE_OK is returned. If an
-** error occurs, an SQLite error code is returned.
+** error occurs, the statement is reset and an SQLite error code is returned.
+**
+** If this function returns SQLITE_ROW, the caller must eventually reset() 
+** statement pSelect. If any other value is returned, the statement does
+** not require a reset().
 **
 ** If the iterator currently points to an INSERT record, bind values from the
 ** new.* record to the SELECT statement. Or, if it points to a DELETE or
@@ -2043,13 +2078,13 @@ static int sessionSeekToRow(
   u8 *abPK,                       /* Primary key flags array */
   sqlite3_stmt *pSelect           /* SELECT statement from sessionSelectRow() */
 ){
-  int rc = SQLITE_OK;             /* Return code */
+  int rc;                         /* Return code */
   int nCol;                       /* Number of columns in table */
   int op;                         /* Changset operation (SQLITE_UPDATE etc.) */
   const char *zDummy;             /* Unused */
 
   sqlite3changeset_op(pIter, &zDummy, &nCol, &op);
-  rc = sessionBindValues(pIter, 
+  rc = sessionBindRow(pIter, 
       op==SQLITE_INSERT ? sqlite3changeset_new : sqlite3changeset_old,
       nCol, abPK, pSelect
   );
@@ -2132,11 +2167,8 @@ static int sessionConflictHandler(
     rc = sqlite3_reset(p->pSelect);
   }else if( rc==SQLITE_OK ){
     /* No other row with the new.* primary key. */
-    rc = sqlite3_reset(p->pSelect);
+    res = xConflict(pCtx, eType+1, pIter);
-    if( rc==SQLITE_OK ){
+    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 && i<nCol; i++){
-      sqlite3_value *pOld = 0;
+      sqlite3_value *pOld = sessionChangesetOld(pIter, i);
-      sqlite3_value *pNew = 0;
+      sqlite3_value *pNew = sessionChangesetNew(pIter, i);
-      rc = sqlite3changeset_old(pIter, i, &pOld);
+
-      if( rc==SQLITE_OK ){
+      sqlite3_bind_int(p->pUpdate, i*3+2, !!pNew);
-        rc = sqlite3changeset_new(pIter, i, &pNew);
+      if( pOld ){
+        rc = sessionBindValue(p->pUpdate, i*3+1, pOld);
       }
-      if( rc==SQLITE_OK ){
+      if( rc==SQLITE_OK && pNew ){
-        if( pOld ) sqlite3_bind_value(p->pUpdate, i*3+1, pOld);
+        rc = sessionBindValue(p->pUpdate, i*3+3, pNew);
-        sqlite3_bind_int(p->pUpdate, i*3+2, !!pNew);
-        if( pNew ) sqlite3_bind_value(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);
       }

manifest

@@ -1,5 +1,5 @@
-C Fix\sa\scouple\stypos\sfor\sconsistency\sin\ssessions\sdocumentation.
+C Add\sOOM\stests\sand\srelated\sfixes\sfor\sthe\ssession\smodule.
-D 2011-03-22T02:03:23.754
+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/session_common.tcl c40e81e86b46adc92b7bcb0182eaa70c8523ad8e
-F ext/session/sessionfault.test da234166d5d044c91964863174d7171b0561708b
+F ext/session/sessionfault.test 2dcf303379d0c01d8320f3c7d0452e6a0dcebd48
-F ext/session/sqlite3session.c 1ca39db8a10b8bcb973b35a68c0924b6a64c4a97
+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
+P 510198f171b9f77a4ad49c06c978c5fbb3a5b7f9
-R 7e8df5e06663856423ec54de25f0cd9f
+R 64f2c206ae40e85e14ca00143e665617
-U shaneh
+U dan
-Z a8ae98277851bb377ac869793dff328e
+Z 4313381608751ccada39ab0745df4a8f

manifest.uuid

@@ -1 +1 @@
-510198f171b9f77a4ad49c06c978c5fbb3a5b7f9
+06048a68b351e3eb15a890cb54db8a1d4b345fbc