Correctly handle I/O errors that occur during OsUnlock(). Before this

fix, an I/O error during OsUnlock() could lead to database corruption.
That is not a serious problem, though, since errors during OsUnlock()
are not possible on most systems. (CVS 4838)

FossilOrigin-Name: b4c1258edb4a40501d13c9da674d0366d5a8c694

manifest

@@ -1,5 +1,5 @@
-C Cleanup\sthe\slocking-style\scode\sin\sos_unix.c.\s(CVS\s4837)
-D 2008-03-07T15:34:11
+C Correctly\shandle\sI/O\serrors\sthat\soccur\sduring\sOsUnlock().\s\sBefore\sthis\nfix,\san\sI/O\serror\sduring\sOsUnlock()\scould\slead\sto\sdatabase\scorruption.\nThat\sis\snot\sa\sserious\sproblem,\sthough,\ssince\serrors\sduring\sOsUnlock()\nare\snot\spossible\son\smost\ssystems.\s(CVS\s4838)
+D 2008-03-07T19:51:14
 F Makefile.arm-wince-mingw32ce-gcc ac5f7b2cef0cd850d6f755ba6ee4ab961b1fadf7
 F Makefile.in d521464011d6965bbda1b699f1850c6e33141c73
 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654
@@ -120,16 +120,16 @@ F src/mutex_unix.c a6e111947a3cdaa2cda394ed060d7f496fcb4af8
 F src/mutex_w32.c 6e197765f283815496193e78e9548b5d0e53b68e
 F src/os.c 2f2753b8d33f79d169c43d6bb0b25b3c58fd33de
 F src/os.h d04706d54a072c7a30ab9e346ad916ef28c842d5
-F src/os_common.h a66179e3b9a2686ba0f17a26d065d3e29202b7b8
+F src/os_common.h e8b748b2f2ecc8a498e50bfe5d8721f189c19d2a
 F src/os_os2.c 10b23539e0050bdfc9f136242086a5c18c70c6f8
 F src/os_os2.h c3f7d0af7e3453d1d7aa81b06c0a56f5a226530b
 F src/os_test.c 49833426101f99aee4bb5f6a44b7c4b2029fda1c
 F src/os_test.h 903c93554c23d88f34f667f1979e4a1cee792af3
-F src/os_unix.c bf339cd689054c8edc603c0209d91f62444c1477
+F src/os_unix.c 04ff58d84ae02d4ef732103455bfe36ce7780738
 F src/os_unix.h 5768d56d28240d3fe4537fac08cc85e4fb52279e
 F src/os_win.c aa3f4bbee3b8c182d25a33fbc319f486857c12c1
 F src/os_win.h 41a946bea10f61c158ce8645e7646b29d44f122b
-F src/pager.c d09885bb88e9868a5c322a0181d4022cf294d98b
+F src/pager.c 803d361f7aabfea1eebf8951916492c77d3781a2
 F src/pager.h 8174615ffd14ccc2cad2b081b919a398fa95e3f9
 F src/parse.y 00f2698c8ae84f315be5e3f10b63c94f531fdd6d
 F src/pragma.c e3f39f8576234887ecd0c1de43dc51af5855930c
@@ -146,7 +146,7 @@ F src/sqliteLimit.h ee4430f88f69bf63527967bb35ca52af7b0ccb1e
 F src/table.c 2c48c575dd59b3a6c5c306bc55f51a9402cf429a
 F src/tclsqlite.c d95e0e74c7167b2807f9f4f73bf45f7c58096297
 F src/test1.c c3d43a6bd299f3c115f6617af6715004819ca5cb
-F src/test2.c 355d5693ca3ee705548fa7f795592a37b2372b70
+F src/test2.c 89793e863188028b49bd259026a73820426539b3
 F src/test3.c 5c7452038ab27aa698070799b10132f26cdd2a80
 F src/test4.c c2c0f5dc907f1346f5d4b65eb5799f11eb9e4071
 F src/test5.c 3a6a5717a149d7ca2e6d14f5be72cf7555d54dc4
@@ -353,7 +353,7 @@ F test/interrupt.test 42e7cf98646fd9cb4a3b131a93ed3c50b9e149f1
 F test/intpkey.test 537669fd535f62632ca64828e435b9e54e8d677f
 F test/io.test f2e9890eb3f159973fcb11008054596b0caeca4f
 F test/ioerr.test 5382046b2ef19b9084652e7d2d1e82ab33c0deda
-F test/ioerr2.test e3d52c40f43f9b61da9b38951a737e7b84ebae96
+F test/ioerr2.test b9c9a0491a812707762a7c002876553be54d9969
 F test/ioerr3.test d3cec5e1a11ad6d27527d0d38573fbff14c71bdd
 F test/join.test af0443185378b64878750aa1cf4b83c216f246b4
 F test/join2.test f2171c265e57ee298a27e57e7051d22962f9f324
@@ -458,7 +458,7 @@ F test/table.test 13b1c2e2fb4727b35ee1fb7641fc469214fd2455
 F test/tableapi.test 4546eb710d979db023bfcc16b0c108b1557fcb43
 F test/tclsqlite.test 3fac87cb1059c46b8fa8a60b553f4f1adb0fb6d9
 F test/temptable.test 19b851b9e3e64d91e9867619b2a3f5fffee6e125
-F test/tester.tcl 7760c4101448e5595b2ee095e540643fa31a1610
+F test/tester.tcl 8f1df98df42667d05ee56ac4ba1ee58f9bb1e885
 F test/thread001.test 8fbd9559da0bbdc273e00318c7fd66c162020af7
 F test/thread002.test 2c4ad2c386f60f6fe268cd91c769ee35b3c1fd0b
 F test/thread1.test 776c9e459b75ba905193b351926ac4019b049f35
@@ -623,7 +623,7 @@ F www/tclsqlite.tcl 8be95ee6dba05eabcd27a9d91331c803f2ce2130
 F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0
 F www/version3.tcl 890248cf7b70e60c383b0e84d77d5132b3ead42b
 F www/whentouse.tcl fc46eae081251c3c181bd79c5faef8195d7991a5
-P 9819cefbd7032fe6884c6c891e8e399000e0821f
-R 94e0b03717c8cb302d6990c16331816e
+P 40f55c09dbd31f861b9f9c7641cce92553d94e35
+R 862dd7b7066eb395b14356fb582df1d4
 U drh
-Z c7afd0fbb52cbc7669e22683bc9d24e9
+Z aa58e6aa52b1bd4468d6e2d048a914af

manifest.uuid

@@ -1 +1 @@
-40f55c09dbd31f861b9f9c7641cce92553d94e35
+b4c1258edb4a40501d13c9da674d0366d5a8c694

src/os_common.h

@@ -86,19 +86,22 @@ static unsigned int elapse;
 ** is used for testing the I/O recovery logic.
 */
 #ifdef SQLITE_TEST
-int sqlite3_io_error_hit = 0;
-int sqlite3_io_error_pending = 0;
-int sqlite3_io_error_persist = 0;
+int sqlite3_io_error_hit = 0;            /* Total number of I/O Errors */
+int sqlite3_io_error_hardhit = 0;        /* Number of non-benign errors */
+int sqlite3_io_error_pending = 0;        /* Count down to first I/O error */
+int sqlite3_io_error_persist = 0;        /* True if I/O errors persist */
+int sqlite3_io_error_benign = 0;         /* True if errors are benign */
 int sqlite3_diskfull_pending = 0;
 int sqlite3_diskfull = 0;
+#define SimulateIOErrorBenign(X) sqlite3_io_error_benign=(X)
 #define SimulateIOError(CODE)  \
-  if( sqlite3_io_error_pending || sqlite3_io_error_hit ) \
-     if( sqlite3_io_error_pending-- == 1 \
-         || (sqlite3_io_error_persist && sqlite3_io_error_hit) ) \
+  if( (sqlite3_io_error_persist && sqlite3_io_error_hit) \
+       || sqlite3_io_error_pending-- == 1 )  \
               { local_ioerr(); CODE; }
 static void local_ioerr(){
   IOTRACE(("IOERR\n"));
-  sqlite3_io_error_hit = 1;
+  sqlite3_io_error_hit++;
+  if( !sqlite3_io_error_benign ) sqlite3_io_error_hardhit++;
 }
 #define SimulateDiskfullError(CODE) \
    if( sqlite3_diskfull_pending ){ \
@@ -112,6 +115,7 @@ static void local_ioerr(){
      } \
    }
 #else
+#define SimulateIOErrorBenign(X)
 #define SimulateIOError(A)
 #define SimulateDiskfullError(A)
 #endif

src/os_unix.c

@@ -1326,6 +1326,7 @@ static int unixUnlock(sqlite3_file *id, int locktype){
   struct flock lock;
   int rc = SQLITE_OK;
   unixFile *pFile = (unixFile*)id;
+  int h;
 
   assert( pFile );
   OSTRACE7("UNLOCK  %d %d was %d(%d,%d) pid=%d\n", pFile->h, locktype,
@@ -1339,17 +1340,20 @@ static int unixUnlock(sqlite3_file *id, int locktype){
     return SQLITE_MISUSE;
   }
   enterMutex();
+  h = pFile->h;
   pLock = pFile->pLock;
   assert( pLock->cnt!=0 );
   if( pFile->locktype>SHARED_LOCK ){
     assert( pLock->locktype==pFile->locktype );
+    SimulateIOErrorBenign(1);
+    SimulateIOError( h=(-1) )
+    SimulateIOErrorBenign(0);
     if( locktype==SHARED_LOCK ){
       lock.l_type = F_RDLCK;
       lock.l_whence = SEEK_SET;
       lock.l_start = SHARED_FIRST;
       lock.l_len = SHARED_SIZE;
-      if( fcntl(pFile->h, F_SETLK, &lock)==(-1) ){
-        /* This should never happen */
+      if( fcntl(h, F_SETLK, &lock)==(-1) ){
         rc = SQLITE_IOERR_RDLOCK;
       }
     }
@@ -1357,10 +1361,10 @@ static int unixUnlock(sqlite3_file *id, int locktype){
     lock.l_whence = SEEK_SET;
     lock.l_start = PENDING_BYTE;
     lock.l_len = 2L;  assert( PENDING_BYTE+1==RESERVED_BYTE );
-    if( fcntl(pFile->h, F_SETLK, &lock)!=(-1) ){
+    if( fcntl(h, F_SETLK, &lock)!=(-1) ){
       pLock->locktype = SHARED_LOCK;
     }else{
-      rc = SQLITE_IOERR_UNLOCK;  /* This should never happen */
+      rc = SQLITE_IOERR_UNLOCK;
     }
   }
   if( locktype==NO_LOCK ){
@@ -1375,10 +1379,14 @@ static int unixUnlock(sqlite3_file *id, int locktype){
       lock.l_type = F_UNLCK;
       lock.l_whence = SEEK_SET;
       lock.l_start = lock.l_len = 0L;
-      if( fcntl(pFile->h, F_SETLK, &lock)!=(-1) ){
+      SimulateIOErrorBenign(1);
+      SimulateIOError( h=(-1) )
+      SimulateIOErrorBenign(0);
+      if( fcntl(h, F_SETLK, &lock)!=(-1) ){
         pLock->locktype = NO_LOCK;
       }else{
-        rc = SQLITE_IOERR_UNLOCK;  /* This should never happen */
+        rc = SQLITE_IOERR_UNLOCK;
+        pLock->cnt = 1;
       }
     }
 
@@ -1386,6 +1394,7 @@ static int unixUnlock(sqlite3_file *id, int locktype){
     ** count reaches zero, close any other file descriptors whose close
     ** was deferred because of outstanding locks.
     */
+    if( rc==SQLITE_OK ){
       pOpen = pFile->pOpen;
       pOpen->nLock--;
       assert( pOpen->nLock>=0 );
@@ -1399,8 +1408,9 @@ static int unixUnlock(sqlite3_file *id, int locktype){
         pOpen->aPending = 0;
       }
     }
+  }
   leaveMutex();
-  pFile->locktype = locktype;
+  if( rc==SQLITE_OK ) pFile->locktype = locktype;
   return rc;
 }
 
@@ -1454,8 +1464,8 @@ static int unixClose(sqlite3_file *id){
  */
 typedef struct afpLockingContext afpLockingContext;
 struct afpLockingContext {
-  unsigned long long sharedLockByte;  /* Byte offset of shared lock byte */
-  const char *filePath;               /* Name of the file */
+  unsigned long long sharedLockByte;
+  char *filePath;
 };
 
 struct ByteRangeLockPB2
@@ -1752,7 +1762,9 @@ static int afpUnixClose(sqlite3_file *id) {
   sqlite3_free(pFile->lockingContext);
   if( pFile->dirfd>=0 ) close(pFile->dirfd);
   pFile->dirfd = -1;
+  enterMutex();
   close(pFile->h);
+  leaveMutex();
   OSTRACE2("CLOSE   %-3d\n", pFile->h);
   OpenCounter(-1);
   memset(pFile, 0, sizeof(unixFile));
@@ -1843,7 +1855,10 @@ static int flockUnixClose(sqlite3_file *id) {
   
   if( pFile->dirfd>=0 ) close(pFile->dirfd);
   pFile->dirfd = -1;
+
+  enterMutex();
   close(pFile->h);  
+  leaveMutex();
   OSTRACE2("CLOSE   %-3d\n", pFile->h);
   OpenCounter(-1);
   memset(pFile, 0, sizeof(unixFile));
@@ -1953,7 +1968,9 @@ static int dotlockUnixClose(sqlite3_file *id) {
   sqlite3_free(pFile->lockingContext);
   if( pFile->dirfd>=0 ) close(pFile->dirfd);
   pFile->dirfd = -1;
+  enterMutex();  
   close(pFile->h);
+  leaveMutex();
   OSTRACE2("CLOSE   %-3d\n", pFile->h);
   OpenCounter(-1);
   memset(pFile, 0, sizeof(unixFile));
@@ -1989,7 +2006,9 @@ static int nolockUnixClose(sqlite3_file *id) {
   if( !pFile ) return SQLITE_OK;
   if( pFile->dirfd>=0 ) close(pFile->dirfd);
   pFile->dirfd = -1;
+  enterMutex();
   close(pFile->h);
+  leaveMutex();
   OSTRACE2("CLOSE   %-3d\n", pFile->h);
   OpenCounter(-1);
   memset(pFile, 0, sizeof(unixFile));

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.412 2008/02/26 18:40:12 drh Exp $
+** @(#) $Id: pager.c,v 1.413 2008/03/07 19:51:14 drh Exp $
 */
 #ifndef SQLITE_OMIT_DISKIO
 #include "sqliteInt.h"
@@ -1241,9 +1241,8 @@ static void pager_reset(Pager *pPager){
 static void pager_unlock(Pager *pPager){
   if( !pPager->exclusiveMode ){
     if( !MEMDB ){
-      if( pPager->fd->pMethods ){
-        osUnlock(pPager->fd, NO_LOCK);
-      }
+      int rc = osUnlock(pPager->fd, NO_LOCK);
+      if( rc ) pager_error(pPager, rc);
       pPager->dbSize = -1;
       IOTRACE(("UNLOCK %p\n", pPager))
 
@@ -1252,7 +1251,7 @@ static void pager_unlock(Pager *pPager){
       ** cache can be discarded and the error code safely cleared.
       */
       if( pPager->errCode ){
-        pPager->errCode = SQLITE_OK;
+        if( rc==SQLITE_OK ) pPager->errCode = SQLITE_OK;
         pager_reset(pPager);
         if( pPager->stmtOpen ){
           sqlite3OsClose(pPager->stfd);
@@ -3384,7 +3383,14 @@ static int pagerSharedLock(Pager *pPager){
         }
         if( rc!=SQLITE_OK ){
           pager_unlock(pPager);
-          return ((rc==SQLITE_NOMEM||rc==SQLITE_IOERR_NOMEM)?rc:SQLITE_BUSY);
+          switch( rc ){
+            case SQLITE_NOMEM:
+            case SQLITE_IOERR_UNLOCK:
+            case SQLITE_IOERR_NOMEM:
+              return rc;
+            default:
+              return SQLITE_BUSY;
+          }
         }
         pPager->journalOpen = 1;
         pPager->journalStarted = 0;

src/test2.c

@@ -13,7 +13,7 @@
 ** is not included in the SQLite library.  It is used for automated
 ** testing of the SQLite library.
 **
-** $Id: test2.c,v 1.53 2008/02/18 14:47:34 drh Exp $
+** $Id: test2.c,v 1.54 2008/03/07 19:51:14 drh Exp $
 */
 #include "sqliteInt.h"
 #include "tcl.h"
@@ -666,6 +666,7 @@ int Sqlitetest2_Init(Tcl_Interp *interp){
   extern int sqlite3_io_error_persist;
   extern int sqlite3_io_error_pending;
   extern int sqlite3_io_error_hit;
+  extern int sqlite3_io_error_hardhit;
   extern int sqlite3_diskfull_pending;
   extern int sqlite3_diskfull;
   extern int sqlite3_pager_n_sort_bucket;
@@ -708,6 +709,8 @@ int Sqlitetest2_Init(Tcl_Interp *interp){
      (char*)&sqlite3_io_error_persist, TCL_LINK_INT);
   Tcl_LinkVar(interp, "sqlite_io_error_hit",
      (char*)&sqlite3_io_error_hit, TCL_LINK_INT);
+  Tcl_LinkVar(interp, "sqlite_io_error_hardhit",
+     (char*)&sqlite3_io_error_hardhit, TCL_LINK_INT);
   Tcl_LinkVar(interp, "sqlite_diskfull_pending",
      (char*)&sqlite3_diskfull_pending, TCL_LINK_INT);
   Tcl_LinkVar(interp, "sqlite_diskfull",

test/ioerr2.test

@@ -15,7 +15,7 @@
 # The tests in this file use special facilities that are only
 # available in the SQLite test fixture.
 #
-# $Id: ioerr2.test,v 1.6 2007/09/12 17:01:45 danielk1977 Exp $
+# $Id: ioerr2.test,v 1.7 2008/03/07 19:51:15 drh Exp $
 
 set testdir [file dirname $argv0]
 source $testdir/tester.tcl
@@ -52,7 +52,7 @@ proc check_db {testname} {
   # connection. Otherwise, try a ROLLBACK, in case a transaction 
   # is still active.
   set rc [catch {execsql {PRAGMA integrity_check}} msg]
-  if {$rc && $msg eq "disk I/O error"} {
+  if {$rc && ($msg eq "disk I/O error" || $msg eq "database is locked")} {
     db close
     sqlite3 db test.db
     set refcnt 0

test/tester.tcl

@@ -11,7 +11,7 @@
 # This file implements some common TCL routines used for regression
 # testing the SQLite library
 #
-# $Id: tester.tcl,v 1.105 2008/02/16 16:21:46 drh Exp $
+# $Id: tester.tcl,v 1.106 2008/03/07 19:51:15 drh Exp $
 
 
 set tcl_precision 15
@@ -526,7 +526,10 @@ proc do_ioerr_test {testname args} {
     # there are at least N IO operations performed by SQLite as
     # a result of the script, the Nth will fail.
     do_test $testname.$n.3 {
+      set ::sqlite_io_error_hit 0
+      set ::sqlite_io_error_hardhit 0
       set r [catch $::ioerrorbody msg]
+      set ::errseen $r
       set rc [sqlite3_errcode $::DB]
       if {$::ioerropts(-erc)} {
         # If we are in extended result code mode, make sure all of the
@@ -558,6 +561,9 @@ proc do_ioerr_test {testname args} {
       }
 
       set s [expr $::sqlite_io_error_hit==0]
+      if {$::sqlite_io_error_hit>$::sqlite_io_error_hardhit && $r==0} {
+        set r 1
+      }
       set ::sqlite_io_error_hit 0
 
       # One of two things must have happened. either
@@ -569,7 +575,7 @@ proc do_ioerr_test {testname args} {
 
     # If an IO error occured, then the checksum of the database should
     # be the same as before the script that caused the IO error was run.
-    if {$::go && $::ioerropts(-cksum)} {
+    if {$::go && $::sqlite_io_error_hardhit && $::ioerropts(-cksum)} {
       do_test $testname.$n.4 {
         catch {db close}
         set ::DB [sqlite3 db test.db; sqlite3_connection_pointer db]
@@ -577,6 +583,7 @@ proc do_ioerr_test {testname args} {
       } $checksum
     }
 
+    set ::sqlite_io_error_hardhit 0
     set ::sqlite_io_error_pending 0
     if {[info exists ::ioerropts(-cleanup)]} {
       catch $::ioerropts(-cleanup)