Fix appendvfs bug exposed with bigger files, and add tests for such conditions.

FossilOrigin-Name: 19b1f53a1c0a14440ae8ac71660a2595d37a4a5b201055c19366c7dca75d6660

diff --git a/ext/misc/appendvfs.c b/ext/misc/appendvfs.c
index 3fff9a10f6a4aa6a1a9edb5bb24b5d83057ff934..970bb282c25d32f1a828be241b1622cccb96a7ca 100644 (file)
--- a/ext/misc/appendvfs.c
+++ b/ext/misc/appendvfs.c
@@ -122,6 +122,10 @@ struct ApndFile {
   sqlite3_int64 iPgOne;
   /* File offset of written append-mark, or -1 if unwritten */
   sqlite3_int64 iMark;
+  /* Whenever file is open and .iMark >= 0, and file size changes are
+   * not in progress, .iMark + sizeof(append mark) == base file size,
+   * and append file size == .iMark - .iPgOne .
+   */
 };
 
 /*
@@ -217,6 +221,7 @@ static const sqlite3_io_methods apnd_io_methods = {
 ** Close an apnd-file.
 */
 static int apndClose(sqlite3_file *pFile){
+  assert((ApndFile *pFile)pFile == ORIGFILE(pFile));
   pFile = ORIGFILE(pFile);
   return pFile->pMethods->xClose(pFile);
 }
@@ -250,6 +255,7 @@ static int apndWriteMark(
   int i = APND_MARK_FOS_SZ;
   int rc;
   assert(pFile == ORIGFILE(paf));
+  /* assert(pFile == paf->base); */
   memcpy(a, APND_MARK_PREFIX, APND_MARK_PREFIX_SZ);
   while (--i >= 0) {
     a[APND_MARK_PREFIX_SZ+i] = (unsigned char)(iPgOne & 0xff);
@@ -276,6 +282,7 @@ static int apndWrite(
   sqlite_int64 iWriteEnd = iOfst + iAmt;
   if( iWriteEnd>=APND_MAX_SIZE ) return SQLITE_FULL;
   pFile = ORIGFILE(pFile);
+  /* assert(pFile == paf->base); */
   /* If append-mark is absent or will be overwritten, write it. */
   if( paf->iMark < 0 || paf->iPgOne + iWriteEnd > paf->iMark ){
     int rc = apndWriteMark(paf, pFile, iWriteEnd);
@@ -499,55 +506,60 @@ static int apndIsOrdinaryDatabaseFile(sqlite3_int64 sz, sqlite3_file *pFile){
 ** Open an apnd file handle.
 */
 static int apndOpen(
-  sqlite3_vfs *pVfs,
+  sqlite3_vfs *pApndVfs,
   const char *zName,
   sqlite3_file *pFile,
   int flags,
   int *pOutFlags
 ){
-  ApndFile *p;
-  sqlite3_file *pSubFile;
-  sqlite3_vfs *pSubVfs;
+  ApndFile *pApndFile = (ApndFile*)pFile;
+  sqlite3_file *pBaseFile = ORIGFILE(pFile);
+  sqlite3_vfs *pBaseVfs = ORIGVFS(pApndVfs);
   int rc;
   sqlite3_int64 sz;
-  pSubVfs = ORIGVFS(pVfs);
   if( (flags & SQLITE_OPEN_MAIN_DB)==0 ){
-    /* The appendvfs is not to be used for transient or temporary databases. */
-    return pSubVfs->xOpen(pSubVfs, zName, pFile, flags, pOutFlags);
+    /* The appendvfs is not to be used for transient or temporary databases.
+     * Just use the base VFS open to initialize the given file object and
+     * open the underlying file. (Appendvfs is then unused for this file.)
+     */
+    return pBaseVfs->xOpen(pBaseVfs, zName, pFile, flags, pOutFlags);
   }
-  p = (ApndFile*)pFile;
-  memset(p, 0, sizeof(*p));
-  pSubFile = ORIGFILE(pFile);
+  memset(pApndFile, 0, sizeof(ApndFile));
   pFile->pMethods = &apnd_io_methods;
-  rc = pSubVfs->xOpen(pSubVfs, zName, pSubFile, flags, pOutFlags);
+  /* Record that append mark has not been written until seen otherwise. */
+  pApndFile->iMark = -1;
+
+  rc = pBaseVfs->xOpen(pBaseVfs, zName, pBaseFile, flags, pOutFlags);
   if( rc ) goto apnd_open_done;
-  rc = pSubFile->pMethods->xFileSize(pSubFile, &sz);
+  rc = pBaseFile->pMethods->xFileSize(pBaseFile, &sz);
   if( rc ){
-    pSubFile->pMethods->xClose(pSubFile);
+    pBaseFile->pMethods->xClose(pBaseFile);
     goto apnd_open_done;
   }
-  if( apndIsOrdinaryDatabaseFile(sz, pSubFile) ){
-    memmove(pFile, pSubFile, pSubVfs->szOsFile);
+  if( apndIsOrdinaryDatabaseFile(sz, pBaseFile) ){
+    /* The file being opened appears to be just an ordinary DB. Copy
+     * the base dispatch-table so this instance mimics the base VFS. 
+     */
+    memmove(pApndFile, pBaseFile, pBaseVfs->szOsFile);
     return SQLITE_OK;
   }
-  /* Record that append mark has not been written until seen otherwise. */
-  p->iMark = -1;
-  p->iPgOne = apndReadMark(sz, pFile);
-  if( p->iPgOne>=0 ){
+  pApndFile->iPgOne = apndReadMark(sz, pFile);
+  if( pApndFile->iPgOne>=0 ){
     /* Append mark was found, infer its offset */
-    p->iMark = sz - p->iPgOne - APND_MARK_SIZE;
+    pApndFile->iMark = sz - APND_MARK_SIZE;
+    /* pApndFile->base = pBaseFile; */
     return SQLITE_OK;
   }
   if( (flags & SQLITE_OPEN_CREATE)==0 ){
-    pSubFile->pMethods->xClose(pSubFile);
+    pBaseFile->pMethods->xClose(pBaseFile);
     rc = SQLITE_CANTOPEN;
   }
   /* Round newly added appendvfs location to #define'd page boundary. 
    * Note that nothing has yet been written to the underlying file.
    * The append mark will be written along with first content write.
-   * Until then, the p->iMark value indicates it is not yet written.
+   * Until then, paf->iMark value indicates it is not yet written.
    */
-  p->iPgOne = APND_START_ROUNDUP(sz, APND_ROUNDUP_BITS);
+  pApndFile->iPgOne = APND_START_ROUNDUP(sz, APND_ROUNDUP_BITS);
 apnd_open_done:
   if( rc ) pFile->pMethods = 0;
   return rc;

diff --git a/manifest b/manifest
index 634081d3be6e80c944b5adff439a72a51532cd17..1c494e65737b1030bd8a6e8f5a4ba3d327662d60 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Create\snew\sbranch\snamed\s"appendvfs_fix"
-D 2021-03-16T04:03:59.658
+C Fix\sappendvfs\sbug\sexposed\swith\sbigger\sfiles,\sand\sadd\stests\sfor\ssuch\sconditions.
+D 2021-03-16T06:41:51.332
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -286,7 +286,7 @@ F ext/lsm1/tool/mklsm1c.tcl f31561bbee5349f0a554d1ad7236ac1991fc09176626f529f607
 F ext/misc/README.md d6dd0fe1d8af77040216798a6a2b0c46c73054d2f0ea544fbbcdccf6f238c240
 F ext/misc/amatch.c e3ad5532799cee9a97647f483f67f43b38796b84b5a8c60594fe782a4338f358
 F ext/misc/anycollseq.c 5ffdfde9829eeac52219136ad6aa7cd9a4edb3b15f4f2532de52f4a22525eddb
-F ext/misc/appendvfs.c 7fff57cd4a5d63758d20a1dd1a96c64c7c123cf48fd98bbd36cfe8b063236e3d
+F ext/misc/appendvfs.c 714118d14b610c00973aae124c1f8c24d338682bdef77bbe8f0cd2ff26952c65
 F ext/misc/blobio.c a867c4c4617f6ec223a307ebfe0eabb45e0992f74dd47722b96f3e631c0edb2a
 F ext/misc/btreeinfo.c d28ce349b40054eaa9473e835837bad7a71deec33ba13e39f963d50933bfa0f9
 F ext/misc/carray.c b75a0f207391038bf1540d3372f482a95c3613511c7c474db51ede1196321c7c
@@ -699,7 +699,7 @@ F test/autoindex4.test 49d3cd791a9baa16fb461d7ea3de80d019a819cf
 F test/autoindex5.test a5d72fe8c217cc0ea356dc6fa06a282a8a3fc53aa807709d79dba07a8f248102
 F test/autovacuum.test 0831cd34e14695d297187f7f6519265e3121c5b0a1720e548e86829e796129e9
 F test/autovacuum_ioerr2.test 8a367b224183ad801e0e24dcb7d1501f45f244b4
-F test/avfs.test 67a27e590b1baae5249bc857d10814a13231dbe2d624c64f2aa78c4ba85afff9
+F test/avfs.test 0c3a38e03cccb0fc3127838462dc05dc3f4c1480d770c084b388304c25de3652
 F test/avtrans.test b7dc25459ecbd86c6fa9c606ee3068f59d81e225118617dcf2bbb6ded2ade89e
 F test/backcompat.test 3e64cedda754c778ef6bbe417b6e7a295e662a4d
 F test/backup.test dd4a5ff756e3df3931dacb1791db0584d4bad989
@@ -1910,10 +1910,7 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P d9f8f488ff9d47fe7bb8838e683bae4fea038f7278ef885ecf292143a0dd88ed
-R 17f134cb27374fc63f087829be8f2d98
-T *branch * appendvfs_fix
-T *sym-appendvfs_fix *
-T -sym-trunk *
+P 026edd601444d86858a503ffc3be17667a62a29f09c001009d9a678400b3b0a1
+R 3f7323614d0b0269053ff308d3cf0c77
 U larrybr
-Z bbee4d88175664ad835a258941f99787
+Z 1150e673d6f8e609bf947bc6b9f00cc8

diff --git a/manifest.uuid b/manifest.uuid
index c53c0630b89504198138932f68230a5382ccd255..1e3cbd1570a00b2163093ff62fa8c62f20a0e231 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-026edd601444d86858a503ffc3be17667a62a29f09c001009d9a678400b3b0a1
\ No newline at end of file
+19b1f53a1c0a14440ae8ac71660a2595d37a4a5b201055c19366c7dca75d6660
\ No newline at end of file

diff --git a/test/avfs.test b/test/avfs.test
index b2daa70a2dc265572799b6a967085e3ba61c38c1..2ebd608baafa13a17579995d47701b47609db86b 100644 (file)
--- a/test/avfs.test
+++ b/test/avfs.test
@@ -20,6 +20,9 @@
 # avfs-2.1. Test that the simple text file retains its initial text.
 # avfs-3.1. Test that the appendvfs can grow and shrink, remaining intact.
 # avfs-3.2. Test that appendvfs is intact after grow/shrink/close/reopen.
+# avfs-3.3. Test that appendvfs can grow by many pages and be written.
+# avfs-3.4. Test that grown appendvfs can be reopened and appear intact.
+# avfs-3.5. Test that much grown appendvfs can shrink and reopen intact.
 # avfs-4.1. Test shell's ability to append to a non-appendvfs file.
 # avfs-4.2. Test shell's ability to append to empty or nonexistent file.
 # avfs-4.3. Test shell's ability to reopen and alter an appendvfs file.
@@ -159,11 +162,12 @@ do_test 2.1 {
 } {Appendee intact.}
 
 # Set of repeatable random integers for a couple tests.
+set ::nrint 50000
 proc rint {v} {
   return [::tcl::mathfunc::int [expr $v * 100000]]
 }
 array set ::randints [list 0 [rint [::tcl::mathfunc::srand 0]]]
-for {set i 1} {$i < 10000} {incr i} {
+for {set i 1} {$i < $::nrint} {incr i} {
   set ::randints($i) [rint [::tcl::mathfunc::rand]]
 }
 
@@ -176,7 +180,7 @@ do_test 3.1 {
     CREATE TABLE ri (i INTEGER);
     BEGIN;
   }
-  for {set i 0} {$i < 10000} {incr i} {
+  for {set i 0} {$i < $::nrint} {incr i} {
     set r $::randints($i)
     set s $::randints([incr i])
     set t $::randints([incr i])
@@ -203,11 +207,11 @@ do_test 3.1 {
   adb close
   set adaSz [file size $::fa]
   set adba [expr ($adbSz + 0.1)/$adaSz]
-  # lappend results $adbSz $adaSz
+  # lappend results $adba
   set results [concat $results [lrange $qr 0 2]]
-  lappend results [expr {$adba > 10.0 && $adba < 20.0}]
+  lappend results [expr {$adba > 10.0}]
   set ::result [join $results " | "]
-} {ok | 10000 | ok | ok | 1}
+} "ok | $::nrint | ok | ok | 1"
 
 do_test 3.2 {
   set results {}
@@ -219,6 +223,60 @@ do_test 3.2 {
   set ::result [join $results " | "]
 } {ok}
 
+# avfs-3.3. Test that appendvfs can grow by many pages and be written.
+do_test 3.3 {
+  set results {}
+  sqlite3 adb "file:$::fa?mode=rw$::vf" -uri 1
+  set npages 300
+  adb eval { BEGIN }
+  while {$npages > 0} {
+    adb eval { INSERT INTO ri VALUES (randomblob(1500)) }
+    incr npages -1
+  }
+  adb eval { COMMIT }
+  adb eval {
+    SELECT integrity_check as ic FROM pragma_integrity_check();
+  } { lappend results $ic }
+  adb close
+  set adaSzr [expr [file size $::fa] / 300.0 / 1500 ]
+  set okSzr [expr $adaSzr > 1.0 && $adaSzr < 1.3 ]
+  lappend results $okSzr
+  set ::result [join $results " | "]
+} {ok | 1}
+
+# avfs-3.4. Test that grown appendvfs can be reopened and appear intact.
+do_test 3.4 {
+  set results {}
+  sqlite3 adb "file:$::fa?mode=rw$::vf" -uri 1
+  adb eval {
+    SELECT integrity_check as ic FROM pragma_integrity_check();
+  } { lappend results $ic }
+  adb close
+  set ::result $ic
+} {ok}
+
+# avfs-3.5. Test that much grown appendvfs can shrink and reopen intact.
+do_test 3.5 {
+  set results {}
+  set adbsz [file size $::fa]
+  sqlite3 adb "file:$::fa?mode=rw$::vf" -uri 1
+  adb eval {
+    DELETE FROM ri WHERE rowid % 8 <> 0;
+    SELECT integrity_check as ic FROM pragma_integrity_check();
+    VACUUM;
+    SELECT integrity_check as ic FROM pragma_integrity_check();
+  } { lappend results $ic }
+  adb close
+  set adasz [file size $::fa]
+  lappend results [expr {$adbsz/$adasz > 5}]
+  sqlite3 adb "file:$::fa?mode=rw$::vf" -uri 1
+  adb eval {
+    SELECT integrity_check as ic FROM pragma_integrity_check();
+  } { lappend results $ic }
+  adb close
+  set ::result [join $results " | "]
+} {ok | ok | 1 | ok}
+
 set ::cliDoesAr [shellDoesAr]
 
 do_test 4.1 {
@@ -332,6 +390,6 @@ do_test 5.2 {
 
 forcedelete $::fa $::fza
 
-unset -nocomplain ::fa ::fza ::tlo ::result ::randints ::cliDoesAr
+unset -nocomplain ::fa ::fza ::tlo ::result ::randints ::nrint ::cliDoesAr
 
 finish_test