From b0ac3e3a17bfd02e19c4e1b4463406768614cd5e Mon Sep 17 00:00:00 2001 From: dan Date: Wed, 16 Jun 2010 10:55:42 +0000 Subject: [PATCH] Fix a memory leak that can occur in os_unix.c if an IO error occurs within the xUnlock method. FossilOrigin-Name: 6c5c04eea1f0e8d61883ee8675c249fbf895dc01 --- manifest | 21 ++++---- manifest.uuid | 2 +- src/os_unix.c | 70 ++++++++++++------------ src/test_vfs.c | 18 ++++++- test/malloc_common.tcl | 37 ++++++------- test/pager1.test | 48 +++++++++++++++++ test/pagerfault.test | 119 +++++++++++++++++++++++++++++++++++++++++ test/permutations.test | 1 + 8 files changed, 247 insertions(+), 69 deletions(-) create mode 100644 test/pagerfault.test diff --git a/manifest b/manifest index 29700ccfc2..08fb6c9489 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Rationalize\sa\scommon\spattern\sin\stcl\stest\scases\sinto\sproc\sdo_multiclient_test. -D 2010-06-15T19:07:42 +C Fix\sa\smemory\sleak\sthat\scan\soccur\sin\sos_unix.c\sif\san\sIO\serror\soccurs\swithin\sthe\sxUnlock\smethod. +D 2010-06-16T10:55:43 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in a5cad1f8f3e021356bfcc6c77dc16f6f1952bbc3 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -154,7 +154,7 @@ F src/os.c 9c4a2f82a50306a33907678ec0187b6ad1486bfe F src/os.h d7775504a51e6e0d40315aa427b3e229ff9ff9ca F src/os_common.h a8f95b81eca8a1ab8593d23e94f8a35f35d4078f F src/os_os2.c 665876d5eec7585226b0a1cf5e18098de2b2da19 -F src/os_unix.c 22bb2a8c1f3bbf65d91505a5f047014258d63c60 +F src/os_unix.c ae173c9f6afaa58b2833a1c95c6cd32021755c42 F src/os_win.c dfde7d33c446e89dd9a277c036f2c4cc564b3138 F src/pager.c 2964185d4356d0dc159b8340e52d2538d32394e5 F src/pager.h ca1f23c0cf137ac26f8908df2427c8b308361efd @@ -209,7 +209,7 @@ F src/test_schema.c 8c06ef9ddb240c7a0fcd31bc221a6a2aade58bf0 F src/test_server.c bbba05c144b5fc4b52ff650a4328027b3fa5fcc6 F src/test_tclvar.c f4dc67d5f780707210d6bb0eb6016a431c04c7fa F src/test_thread.c aa9919c885a1fe53eafc73492f0898ee6c0a0726 -F src/test_vfs.c db0f5c7c814bde2c6d5df39c900a45929f2f6635 +F src/test_vfs.c 001c34e08748a4a02cd1c2d5531c160a007a84d8 F src/test_wsd.c 41cadfd9d97fe8e3e4e44f61a4a8ccd6f7ca8fe9 F src/tokenize.c 25ceb0f0a746ea1d0f9553787f3f0a56853cfaeb F src/trigger.c 8927588cb9e6d47f933b53bfe74200fbb504100d @@ -508,7 +508,7 @@ F test/mallocH.test 79b65aed612c9b3ed2dcdaa727c85895fd1bfbdb F test/mallocI.test e3ea401904d010cb7c1e4b2ee8803f4a9f5b999d F test/mallocJ.test b5d1839da331d96223e5f458856f8ffe1366f62e F test/mallocK.test d79968641d1b70d88f6c01bdb9a7eb4a55582cc9 -F test/malloc_common.tcl 9b58ffd50d073dccf0493e3ca4aa39bc64ce3047 +F test/malloc_common.tcl fbf369eb2828825c5f319c101917aff91ea87556 F test/manydb.test b3d3bc4c25657e7f68d157f031eb4db7b3df0d3c F test/memdb.test 0825155b2290e900264daaaf0334b6dfe69ea498 F test/memleak.test 10b9c6c57e19fc68c32941495e9ba1c50123f6e2 @@ -533,12 +533,13 @@ F test/notify2.test 195a467e021f74197be2c4fb02d6dee644b8d8db F test/notnull.test cc7c78340328e6112a13c3e311a9ab3127114347 F test/null.test a8b09b8ed87852742343b33441a9240022108993 F test/openv2.test af02ed0a9cbc0d2a61b8f35171d4d117e588e4ec -F test/pager1.test fd1ca712732c3f760f122003497eda8c7886e425 +F test/pager1.test 4e75fc0ebe91d3f96d929098048dedbd67fdc16f +F test/pagerfault.test 16e560bc4332d5b089b369d82ae4b65b8805b5eb F test/pageropt.test 8146bf448cf09e87bb1867c2217b921fb5857806 F test/pagesize.test 76aa9f23ecb0741a4ed9d2e16c5fa82671f28efb F test/pcache.test eebc4420b37cb07733ae9b6e99c9da7c40dd6d58 F test/pcache2.test 0d85f2ab6963aee28c671d4c71bec038c00a1d16 -F test/permutations.test 339011035e1113ab166975572d9333b9eb690d2b +F test/permutations.test f044eaba204ff13d530ceb72a22b0ed2c43562ef F test/pragma.test 6960f9efbce476f70ba9ee2171daf5042f9e3d8a F test/pragma2.test 5364893491b9231dd170e3459bfc2e2342658b47 F test/printf.test 05970cde31b1a9f54bd75af60597be75a5c54fea @@ -821,7 +822,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f -P c1c9f6fa9d75df740e577dbc5e6a24b91ad2bdd0 -R ed40c2790bbf762939a14a6621b8de02 +P efe44564983f115017658dd8a130226366d42bab +R b69e2f7f93141c1bf41706b2581f08b9 U dan -Z ccb2459886575665e681afcce6541b33 +Z cce4c5d034089301ab79e2d9334d049f diff --git a/manifest.uuid b/manifest.uuid index a4df304bc1..0747075dbd 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -efe44564983f115017658dd8a130226366d42bab \ No newline at end of file +6c5c04eea1f0e8d61883ee8675c249fbf895dc01 \ No newline at end of file diff --git a/src/os_unix.c b/src/os_unix.c index 9dbd3d3aad..574f48d189 100644 --- a/src/os_unix.c +++ b/src/os_unix.c @@ -735,18 +735,50 @@ struct unixInodeInfo { */ static unixInodeInfo *inodeList = 0; +/* +** Close all file descriptors accumuated in the unixInodeInfo->pUnused list. +** If all such file descriptors are closed without error, the list is +** cleared and SQLITE_OK returned. +** +** Otherwise, if an error occurs, then successfully closed file descriptor +** entries are removed from the list, and SQLITE_IOERR_CLOSE returned. +** not deleted and SQLITE_IOERR_CLOSE returned. +*/ +static int closePendingFds(unixFile *pFile){ + int rc = SQLITE_OK; + unixInodeInfo *pInode = pFile->pInode; + UnixUnusedFd *pError = 0; + UnixUnusedFd *p; + UnixUnusedFd *pNext; + for(p=pInode->pUnused; p; p=pNext){ + pNext = p->pNext; + if( close(p->fd) ){ + pFile->lastErrno = errno; + rc = SQLITE_IOERR_CLOSE; + p->pNext = pError; + pError = p; + }else{ + sqlite3_free(p); + } + } + pInode->pUnused = pError; + return rc; +} + /* ** Release a unixInodeInfo structure previously allocated by findInodeInfo(). ** ** The mutex entered using the unixEnterMutex() function must be held ** when this function is called. */ -static void releaseInodeInfo(unixInodeInfo *pInode){ +static void releaseInodeInfo(unixFile *pFile){ + unixInodeInfo *pInode = pFile->pInode; assert( unixMutexHeld() ); if( pInode ){ pInode->nRef--; if( pInode->nRef==0 ){ assert( pInode->pShmNode==0 ); + closePendingFds(pFile); if( pInode->pPrev ){ assert( pInode->pPrev->pNext==pInode ); pInode->pPrev->pNext = pInode->pNext; @@ -1152,36 +1184,6 @@ end_lock: return rc; } -/* -** Close all file descriptors accumuated in the unixInodeInfo->pUnused list. -** If all such file descriptors are closed without error, the list is -** cleared and SQLITE_OK returned. -** -** Otherwise, if an error occurs, then successfully closed file descriptor -** entries are removed from the list, and SQLITE_IOERR_CLOSE returned. -** not deleted and SQLITE_IOERR_CLOSE returned. -*/ -static int closePendingFds(unixFile *pFile){ - int rc = SQLITE_OK; - unixInodeInfo *pInode = pFile->pInode; - UnixUnusedFd *pError = 0; - UnixUnusedFd *p; - UnixUnusedFd *pNext; - for(p=pInode->pUnused; p; p=pNext){ - pNext = p->pNext; - if( close(p->fd) ){ - pFile->lastErrno = errno; - rc = SQLITE_IOERR_CLOSE; - p->pNext = pError; - pError = p; - }else{ - sqlite3_free(p); - } - } - pInode->pUnused = pError; - return rc; -} - /* ** Add the file descriptor used by file handle pFile to the corresponding ** pUnused list. @@ -1451,7 +1453,7 @@ static int unixClose(sqlite3_file *id){ */ setPendingFd(pFile); } - releaseInodeInfo(pFile->pInode); + releaseInodeInfo(pFile); rc = closeUnixFile(id); unixLeaveMutex(); } @@ -2066,7 +2068,7 @@ static int semClose(sqlite3_file *id) { semUnlock(id, NO_LOCK); assert( pFile ); unixEnterMutex(); - releaseInodeInfo(pFile->pInode); + releaseInodeInfo(pFile); unixLeaveMutex(); closeUnixFile(id); } @@ -2533,7 +2535,7 @@ static int afpClose(sqlite3_file *id) { */ setPendingFd(pFile); } - releaseInodeInfo(pFile->pInode); + releaseInodeInfo(pFile); sqlite3_free(pFile->lockingContext); rc = closeUnixFile(id); unixLeaveMutex(); diff --git a/src/test_vfs.c b/src/test_vfs.c index ef673c271f..e94bef7d29 100644 --- a/src/test_vfs.c +++ b/src/test_vfs.c @@ -76,7 +76,8 @@ struct Testvfs { #define TESTVFS_OPEN_MASK 0x00000100 #define TESTVFS_SYNC_MASK 0x00000200 -#define TESTVFS_ALL_MASK 0x000003FF +#define TESTVFS_DELETE_MASK 0x00000400 +#define TESTVFS_ALL_MASK 0x000007FF #define TESTVFS_MAX_PAGES 256 @@ -457,7 +458,19 @@ static int tvfsOpen( ** returning. */ static int tvfsDelete(sqlite3_vfs *pVfs, const char *zPath, int dirSync){ - return sqlite3OsDelete(PARENTVFS(pVfs), zPath, dirSync); + int rc = SQLITE_OK; + Testvfs *p = (Testvfs *)pVfs->pAppData; + + if( p->pScript && p->mask&TESTVFS_DELETE_MASK ){ + tvfsExecTcl(p, "xDelete", + Tcl_NewStringObj(zPath, -1), Tcl_NewIntObj(dirSync), 0 + ); + tvfsResultCode(p, &rc); + } + if( rc==SQLITE_OK ){ + rc = sqlite3OsDelete(PARENTVFS(pVfs), zPath, dirSync); + } + return rc; } /* @@ -843,6 +856,7 @@ static int testvfs_obj_cmd( { "xShmClose", TESTVFS_SHMCLOSE_MASK }, { "xShmMap", TESTVFS_SHMMAP_MASK }, { "xSync", TESTVFS_SYNC_MASK }, + { "xDelete", TESTVFS_DELETE_MASK }, { "xOpen", TESTVFS_OPEN_MASK }, }; Tcl_Obj **apElem = 0; diff --git a/test/malloc_common.tcl b/test/malloc_common.tcl index 902f683104..a24716cd4a 100644 --- a/test/malloc_common.tcl +++ b/test/malloc_common.tcl @@ -109,36 +109,29 @@ proc do_faultsim_test {name args} { #------------------------------------------------------------------------- # Procedures to save and restore the current file-system state: # +# faultsim_save # faultsim_save_and_close # faultsim_restore_and_reopen +# faultsim_delete_and_reopen # -proc faultsim_save_and_close {} { - foreach {a => b} { - test.db => testX.db - test.db-wal => testX.db-wal - test.db-journal => testX.db-journal - } { - if {[file exists $a]} { - file copy -force $a $b - } else { - file delete -force $b - } +proc faultsim_save {} { + foreach f [glob -nocomplain sv_test.db*] { file delete -force $f } + foreach f [glob -nocomplain test.db*] { + set f2 "sv_$f" + file copy -force $f $f2 } +} +proc faultsim_save_and_close {} { + faultsim_save catch { db close } return "" } proc faultsim_restore_and_reopen {} { catch { db close } - foreach {a => b} { - testX.db => test.db - testX.db-wal => test.db-wal - testX.db-journal => test.db-journal - } { - if {[file exists $a]} { - file copy -force $a $b - } else { - file delete -force $b - } + foreach f [glob -nocomplain test.db*] { file delete -force $f } + foreach f2 [glob -nocomplain sv_test.db*] { + set f [string range $f2 3 end] + file copy -force $f2 $f } sqlite3 db test.db sqlite3_extended_result_codes db 1 @@ -152,7 +145,7 @@ proc faultsim_integrity_check {{db db}} { proc faultsim_delete_and_reopen {{file test.db}} { catch { db close } - file delete -force test.db test.db-wal test.db-journal + foreach f [glob -nocomplain test.db*] { file delete -force $f } sqlite3 db test.db } diff --git a/test/pager1.test b/test/pager1.test index 51a4da6ec4..e99bf0ed7f 100644 --- a/test/pager1.test +++ b/test/pager1.test @@ -15,6 +15,21 @@ source $testdir/tester.tcl source $testdir/lock_common.tcl source $testdir/malloc_common.tcl +# +# pager1-1.*: Test inter-process locking (clients in multiple processes). +# +# pager1-2.*: Test intra-process locking (multiple clients in this process). +# +# pager1-3.*: Savepoint related tests. +# + +proc do_execsql_test {testname sql result} { + uplevel do_test $testname [list "execsql {$sql}"] [list $result] +} +proc do_catchsql_test {testname sql result} { + uplevel do_test $testname [list "catchsql {$sql}"] [list $result] +} + do_multiclient_test tn { # Create and populate a database table using connection [db]. Check @@ -151,5 +166,38 @@ do_multiclient_test tn { do_test pager1-$tn.28 { sql3 { SELECT * FROM t1 } } {21 one 22 two 23 three} } +do_test pager1-3.1 { + faultsim_delete_and_reopen + execsql { + CREATE TABLE t1(a PRIMARY KEY, b); + CREATE TABLE counter( + i CHECK (i<5), + u CHECK (u<10) + ); + INSERT INTO counter VALUES(0, 0); + CREATE TRIGGER tr1 AFTER INSERT ON t1 BEGIN + UPDATE counter SET i = i+1; + END; + CREATE TRIGGER tr2 AFTER UPDATE ON t1 BEGIN + UPDATE counter SET u = u+1; + END; + } + execsql { SELECT * FROM counter } +} {0 0} + +do_execsql_test pager1-3.2 { + BEGIN; + INSERT INTO t1 VALUES(1, randomblob(1500)); + INSERT INTO t1 VALUES(2, randomblob(1500)); + INSERT INTO t1 VALUES(3, randomblob(1500)); + SELECT * FROM counter; +} {3 0} +do_catchsql_test pager1-3.3 { + INSERT INTO t1 SELECT a+3, randomblob(1500) FROM t1 +} {1 {constraint failed}} +do_execsql_test pager1-3.4 { SELECT * FROM counter } {3 0} +do_execsql_test pager1-3.5 { SELECT a FROM t1 } {1 2 3} +do_execsql_test pager1-3.6 { COMMIT } {} + finish_test diff --git a/test/pagerfault.test b/test/pagerfault.test new file mode 100644 index 0000000000..8dc99e0bc0 --- /dev/null +++ b/test/pagerfault.test @@ -0,0 +1,119 @@ +# 2010 June 15 +# +# The author disclaims copyright to this source code. In place of +# a legal notice, here is a blessing: +# +# May you do good and not evil. +# May you find forgiveness for yourself and forgive others. +# May you share freely, never taking more than you give. +# +#*********************************************************************** +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +source $testdir/lock_common.tcl +source $testdir/malloc_common.tcl + +set a_string_counter 1 +proc a_string {n} { + global a_string_counter + incr a_string_counter + string range [string repeat "${a_string_counter}." $n] 1 $n +} +db func a_string a_string + +#------------------------------------------------------------------------- +# Test fault-injection while rolling back a hot-journal file. +# +do_test pagerfault-1-pre1 { + execsql { + PRAGMA journal_mode = DELETE; + PRAGMA cache_size = 10; + CREATE TABLE t1(a UNIQUE, b UNIQUE); + INSERT INTO t1 VALUES(a_string(200), a_string(300)); + INSERT INTO t1 SELECT a_string(200), a_string(300) FROM t1; + INSERT INTO t1 SELECT a_string(200), a_string(300) FROM t1; + BEGIN; + INSERT INTO t1 SELECT a_string(201), a_string(301) FROM t1; + INSERT INTO t1 SELECT a_string(202), a_string(302) FROM t1; + INSERT INTO t1 SELECT a_string(203), a_string(303) FROM t1; + INSERT INTO t1 SELECT a_string(204), a_string(304) FROM t1; + } + faultsim_save_and_close +} {} +do_faultsim_test pagerfault-1 -prep { + faultsim_restore_and_reopen +} -body { + execsql { SELECT count(*) FROM t1 } +} -test { + faultsim_test_result {0 4} + faultsim_integrity_check + if {[db one { SELECT count(*) FROM t1 }] != 4} { + error "Database content appears incorrect" + } +} + +#------------------------------------------------------------------------- +# Test fault-injection while rolling back hot-journals that were created +# as part of a multi-file transaction. +# +do_test pagerfault-2-pre1 { + testvfs tstvfs -default 1 + tstvfs filter xDelete + tstvfs script xDeleteCallback + + proc xDeleteCallback {method file args} { + set file [file tail $file] + if { [string match *mj* $file] } { faultsim_save } + } + + faultsim_delete_and_reopen + db func a_string a_string + + execsql { + ATTACH 'test.db2' AS aux; + PRAGMA journal_mode = DELETE; + PRAGMA main.cache_size = 10; + PRAGMA aux.cache_size = 10; + + CREATE TABLE t1(a UNIQUE, b UNIQUE); + CREATE TABLE aux.t2(a UNIQUE, b UNIQUE); + INSERT INTO t1 VALUES(a_string(200), a_string(300)); + INSERT INTO t1 SELECT a_string(200), a_string(300) FROM t1; + INSERT INTO t1 SELECT a_string(200), a_string(300) FROM t1; + INSERT INTO t2 SELECT * FROM t1; + + BEGIN; + INSERT INTO t1 SELECT a_string(201), a_string(301) FROM t1; + INSERT INTO t1 SELECT a_string(202), a_string(302) FROM t1; + INSERT INTO t1 SELECT a_string(203), a_string(303) FROM t1; + INSERT INTO t1 SELECT a_string(204), a_string(304) FROM t1; + REPLACE INTO t2 SELECT * FROM t1; + COMMIT; + } + + db close + tstvfs delete +} {} +do_faultsim_test pagerfault-2 -faults ioerr-persistent -prep { + faultsim_restore_and_reopen +} -body { + execsql { + ATTACH 'test.db2' AS aux; + SELECT count(*) FROM t2; + SELECT count(*) FROM t1; + } +} -test { + faultsim_test_result {0 {4 4}} {1 {unable to open database: test.db2}} + faultsim_integrity_check + + catchsql { ATTACH 'test.db2' AS aux } + if {[db one { SELECT count(*) FROM t1 }] != 4 + || [db one { SELECT count(*) FROM t2 }] != 4 + } { + error "Database content appears incorrect" + } +} + +finish_test diff --git a/test/permutations.test b/test/permutations.test index fef111e770..921df33a47 100644 --- a/test/permutations.test +++ b/test/permutations.test @@ -169,6 +169,7 @@ test_suite "coverage-pager" -description { Coverage tests for file pager.c. } -files { pager1.test + pagerfault.test }