From: drh Date: Fri, 7 Mar 2008 19:51:14 +0000 (+0000) Subject: Correctly handle I/O errors that occur during OsUnlock(). Before this X-Git-Tag: version-3.6.10~1339 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1aa5af1151e4466cef3c1520a17a3128fe939b37;p=thirdparty%2Fsqlite.git 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 --- diff --git a/manifest b/manifest index bfad0762d5..2ac532c6fc 100644 --- a/manifest +++ b/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 diff --git a/manifest.uuid b/manifest.uuid index 72f4e16818..3f780d8c89 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -40f55c09dbd31f861b9f9c7641cce92553d94e35 \ No newline at end of file +b4c1258edb4a40501d13c9da674d0366d5a8c694 \ No newline at end of file diff --git a/src/os_common.h b/src/os_common.h index 1efc61c53e..26ea7d6af4 100644 --- a/src/os_common.h +++ b/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) ) \ - { local_ioerr(); CODE; } + 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 diff --git a/src/os_unix.c b/src/os_unix.c index df30a1cc2b..47f3020c7d 100644 --- a/src/os_unix.c +++ b/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,21 +1394,23 @@ static int unixUnlock(sqlite3_file *id, int locktype){ ** count reaches zero, close any other file descriptors whose close ** was deferred because of outstanding locks. */ - pOpen = pFile->pOpen; - pOpen->nLock--; - assert( pOpen->nLock>=0 ); - if( pOpen->nLock==0 && pOpen->nPending>0 ){ - int i; - for(i=0; inPending; i++){ - close(pOpen->aPending[i]); + if( rc==SQLITE_OK ){ + pOpen = pFile->pOpen; + pOpen->nLock--; + assert( pOpen->nLock>=0 ); + if( pOpen->nLock==0 && pOpen->nPending>0 ){ + int i; + for(i=0; inPending; i++){ + close(pOpen->aPending[i]); + } + free(pOpen->aPending); + pOpen->nPending = 0; + pOpen->aPending = 0; } - free(pOpen->aPending); - pOpen->nPending = 0; - 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)); diff --git a/src/pager.c b/src/pager.c index 9f0030aa1b..18fcda1292 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.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; diff --git a/src/test2.c b/src/test2.c index 4d7031dbcc..153806cded 100644 --- a/src/test2.c +++ b/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", diff --git a/test/ioerr2.test b/test/ioerr2.test index ff72f82177..567e5fcbec 100644 --- a/test/ioerr2.test +++ b/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 diff --git a/test/tester.tcl b/test/tester.tcl index d5b5ae5116..5b5935207e 100644 --- a/test/tester.tcl +++ b/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)