From 4245c405ea9ef9115dd2dd6b81afc10a6edf67d1 Mon Sep 17 00:00:00 2001 From: drh Date: Sat, 2 Jun 2012 14:32:21 +0000 Subject: [PATCH] The sqlite3_close() interface returns SQLITE_OK even if there are outstanding sqlite3_stmt and sqlite3_backup objects. The connection becomes a zombie. Resource deallocation is deferred until the last sqlite3_stmt or sqlite3_backup object closes. This is intended to help SQLite play nicer with garbage collectors. FossilOrigin-Name: e276a02b7f54e804caa553dca99023416a415e1c --- manifest | 33 ++++++++++++++++-------------- manifest.uuid | 2 +- src/backup.c | 7 +++---- src/main.c | 53 ++++++++++++++++++++++++++++++++++-------------- src/sqlite.h.in | 13 +++++++----- src/sqliteInt.h | 2 ++ src/vdbeapi.c | 11 +++------- src/vdbeaux.c | 1 + test/backup.test | 9 +++----- test/capi3.test | 10 ++------- test/capi3c.test | 16 ++++----------- test/misuse.test | 2 +- 12 files changed, 84 insertions(+), 75 deletions(-) diff --git a/manifest b/manifest index b001b92c56..bed498a322 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Avoid\scalling\sfchown()\sif\sthe\sprocess\sis\snot\srunning\sas\sroot. -D 2012-05-31T13:10:49.376 +C The\ssqlite3_close()\sinterface\sreturns\sSQLITE_OK\seven\sif\sthere\sare\soutstanding\nsqlite3_stmt\sand\ssqlite3_backup\sobjects.\s\sThe\sconnection\sbecomes\sa\szombie.\nResource\sdeallocation\sis\sdeferred\suntil\sthe\slast\ssqlite3_stmt\sor\s\nsqlite3_backup\sobject\scloses.\s\sThis\sis\sintended\sto\shelp\sSQLite\splay\snicer\nwith\sgarbage\scollectors. +D 2012-06-02T14:32:21.619 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in 4f37eb61be9d38643cdd839a74b8e3bad724cfcf F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -120,7 +120,7 @@ F src/alter.c 149cc80d9257971b0bff34e58fb2263e01998289 F src/analyze.c 70c46504c0d2543ea5cdca01140b2cd3e1d886e7 F src/attach.c 12c6957996908edc31c96d7c68d4942c2474405f F src/auth.c 523da7fb4979469955d822ff9298352d6b31de34 -F src/backup.c 6be23a344d3301ae38e92fddb3a33b91c309fce4 +F src/backup.c 641eab30db6c70c3393719c4c874ecf534eafb27 F src/bitvec.c af50f1c8c0ff54d6bdb7a80e2fceca5a93670bef F src/btmutex.c 976f45a12e37293e32cae0281b15a21d48a8aaa7 F src/btree.c f0b71054103cb77eb5e782088c16998ec4f06624 @@ -145,7 +145,7 @@ F src/journal.c 552839e54d1bf76fb8f7abe51868b66acacf6a0e F src/legacy.c a199d7683d60cef73089e892409113e69c23a99f F src/lempar.c 0ee69fca0be54cd93939df98d2aca4ca46f44416 F src/loadext.c f20382fbaeec832438a1ba7797bee3d3c8a6d51d -F src/main.c fdd95737fe28db3ad2b740c8a134b40592c6b20a +F src/main.c 7ccc7d2dbd06b8eb9226e3f4f46150299dc69da3 F src/malloc.c fe085aa851b666b7c375c1ff957643dc20a04bf6 F src/mem0.c 6a55ebe57c46ca1a7d98da93aaa07f99f1059645 F src/mem1.c b3677415e69603d6a0e7c5410a1b3731d55beda1 @@ -180,9 +180,9 @@ F src/resolve.c b3c70ab28cac60de33684c9aa9e5138dcf71d6dd F src/rowset.c f6a49f3e9579428024662f6e2931832511f831a1 F src/select.c f6c4833c4d8e94714761d99013d74f381e084f1d F src/shell.c c16f72e34f611f060546709564c121a67cb2b31b -F src/sqlite.h.in 922d2907cc2b0177b2c4a3b462f04937750d6edd +F src/sqlite.h.in f8f4b07ecf9516403ecb0b88c2c4e6fe939bd495 F src/sqlite3ext.h 6904f4aadf976f95241311fbffb00823075d9477 -F src/sqliteInt.h 64cffc7ff43725ade0cdfdbab0ef6a56ef3b682f +F src/sqliteInt.h d3b7409a510f7aeb0f9e6a2c74a18db3c659435c F src/sqliteLimit.h 164b0e6749d31e0daa1a4589a169d31c0dec7b3d F src/status.c 35939e7e03abf1b7577ce311f48f682c40de3208 F src/table.c 2cd62736f845d82200acfa1287e33feb3c15d62e @@ -242,8 +242,8 @@ F src/vacuum.c bfd53f9bd20a8fdb70b0fa8e77182b866875c0d8 F src/vdbe.c b6cb2ac43263843a5612892c0ad2309609b32c26 F src/vdbe.h 18f581cac1f4339ec3299f3e0cc6e11aec654cdb F src/vdbeInt.h 6ff4180a05683566a8835d12f7ec504b22932c82 -F src/vdbeapi.c 3662b6a468a2a4605a15dfab313baa6dff81ad91 -F src/vdbeaux.c d52c8a424fdd4b1d5cf1ac93cc7cd20da023ec5c +F src/vdbeapi.c f8ba09132fe654ffd068058cef490426aca9fca6 +F src/vdbeaux.c dce80038c3c41f2680e5ab4dd0f7e0d8b7ff9071 F src/vdbeblob.c 32f2a4899d67f69634ea4dd93e3f651936d732cb F src/vdbemem.c cb55e84b8e2c15704968ee05f0fae25883299b74 F src/vdbesort.c b25814d385895544ebc8118245c8311ded7f81c9 @@ -288,7 +288,7 @@ F test/autovacuum.test fcaf4616ae5bb18098db1cb36262565e5c841c3c F test/autovacuum_ioerr2.test 8a367b224183ad801e0e24dcb7d1501f45f244b4 F test/avtrans.test 0252654f4295ddda3b2cce0e894812259e655a85 F test/backcompat.test bccbc64769d9c755ad65ee7c2f7336b86e3cc0c8 -F test/backup.test 717346db953e9e435c2a94916e4af177330d60d3 +F test/backup.test d7c3e3d522631c3e44fddeac1e5f2f8cb5498a2c F test/backup2.test 34986ef926ea522911a51dfdb2f8e99b7b75ebcf F test/backup_ioerr.test 40d208bc9224b666ee3ed423f49bc9062a36a9d0 F test/backup_malloc.test 7162d604ec2b4683c4b3799a48657fb8b5e2d450 @@ -314,9 +314,9 @@ F test/boundary4.test 89e02fa66397b8a325d5eb102b5806f961f8ec4b F test/busy.test 76b4887f8b9160ba903c1ac22e8ff406ad6ae2f0 F test/cache.test f64136b0893c293d0b910ed057b3b711249099a7 F test/capi2.test 835d4cee9f542ea50fa8d01f3fe6de80b0627360 -F test/capi3.test 8dedb0050610e9ff95cd9d487beb0ce5f33a31ee +F test/capi3.test d527782c154b1dc8a96d16c140226dcf3cff0834 F test/capi3b.test efb2b9cfd127efa84433cd7a2d72ce0454ae0dc4 -F test/capi3c.test 01f197d73f4d4d66316483662f475cab7ab5bd60 +F test/capi3c.test 51b51d053549c9a022cf3c04f334e697f44fbd45 F test/capi3d.test 17b57ca28be3e37e14c2ba8f787d292d84b724a1 F test/capi3e.test f7408dda65c92b9056199fdc180f893015f83dde F test/cast.test 4c275cbdc8202d6f9c54a3596701719868ac7dc3 @@ -618,7 +618,7 @@ F test/misc4.test 9c078510fbfff05a9869a0b6d8b86a623ad2c4f6 F test/misc5.test 528468b26d03303b1f047146e5eefc941b9069f5 F test/misc6.test 953cc693924d88e6117aeba16f46f0bf5abede91 F test/misc7.test 4337d84e441f36cee62656f9f7ba8bc22a7ca721 -F test/misuse.test ba4fb5d1a6101d1c171ea38b3c613d0661c83054 +F test/misuse.test 1564457e771fa0d1066b9a29b1c3149837f8c561 F test/multiplex.test e08cc7177bd6d85990ee1d71100bb6c684c02256 F test/multiplex2.test 580ca5817c7edbe4cc68fa150609c9473393003a F test/multiplex3.test d228f59eac91839a977eac19f21d053f03e4d101 @@ -1004,7 +1004,10 @@ F tool/tostr.awk e75472c2f98dd76e06b8c9c1367f4ab07e122d06 F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f F tool/warnings-clang.sh a8a0a3babda96dfb1ff51adda3cbbf3dfb7266c2 F tool/warnings.sh fbc018d67fd7395f440c28f33ef0f94420226381 -P 07935d10d341fe6265cfd3b09e2c4ef4005c4826 -R db0f48be730e02610af744a3fe432e51 +P 70c419a434be77b042a23174483d6a411899eb5d +R 117d6304ef757a98fccc6a61da6ef5e1 +T *branch * deferred-close +T *sym-deferred-close * +T -sym-trunk * U drh -Z a3e118b76a7afa02078e1c319e3927b5 +Z e1d9ca4a10dedd659e87ebf966dd6be6 diff --git a/manifest.uuid b/manifest.uuid index b74e97b330..d79db39f4e 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -70c419a434be77b042a23174483d6a411899eb5d \ No newline at end of file +e276a02b7f54e804caa553dca99023416a415e1c \ No newline at end of file diff --git a/src/backup.c b/src/backup.c index 7a4047f34c..6655ee3916 100644 --- a/src/backup.c +++ b/src/backup.c @@ -543,14 +543,13 @@ int sqlite3_backup_step(sqlite3_backup *p, int nPage){ */ int sqlite3_backup_finish(sqlite3_backup *p){ sqlite3_backup **pp; /* Ptr to head of pagers backup list */ - MUTEX_LOGIC( sqlite3_mutex *mutex; ) /* Mutex to protect source database */ + sqlite3 *pSrcDb = p->pSrcDb; /* Source database connection */ int rc; /* Value to return */ /* Enter the mutexes */ if( p==0 ) return SQLITE_OK; sqlite3_mutex_enter(p->pSrcDb->mutex); sqlite3BtreeEnter(p->pSrc); - MUTEX_LOGIC( mutex = p->pSrcDb->mutex; ) if( p->pDestDb ){ sqlite3_mutex_enter(p->pDestDb->mutex); } @@ -576,7 +575,7 @@ int sqlite3_backup_finish(sqlite3_backup *p){ /* Exit the mutexes and free the backup context structure. */ if( p->pDestDb ){ - sqlite3_mutex_leave(p->pDestDb->mutex); + sqlite3LeaveMutexAndCloseZombie(p->pDestDb); } sqlite3BtreeLeave(p->pSrc); if( p->pDestDb ){ @@ -585,7 +584,7 @@ int sqlite3_backup_finish(sqlite3_backup *p){ ** sqlite3_backup_finish(). */ sqlite3_free(p); } - sqlite3_mutex_leave(mutex); + sqlite3LeaveMutexAndCloseZombie(pSrcDb); return rc; } diff --git a/src/main.c b/src/main.c index 4e9a605f40..d0aebd5eee 100644 --- a/src/main.c +++ b/src/main.c @@ -703,6 +703,7 @@ void sqlite3CloseSavepoints(sqlite3 *db){ db->isTransactionSavepoint = 0; } + /* ** Invoke the destructor function associated with FuncDef p, if any. Except, ** if this is not the last copy of the function, do not invoke it. Multiple @@ -724,9 +725,6 @@ static void functionDestroy(sqlite3 *db, FuncDef *p){ ** Close an existing SQLite database */ int sqlite3_close(sqlite3 *db){ - HashElem *i; /* Hash table iterator */ - int j; - if( !db ){ return SQLITE_OK; } @@ -747,25 +745,51 @@ int sqlite3_close(sqlite3 *db){ */ sqlite3VtabRollback(db); - /* If there are any outstanding VMs, return SQLITE_BUSY. */ - if( db->pVdbe ){ - sqlite3Error(db, SQLITE_BUSY, - "unable to close due to unfinalised statements"); - sqlite3_mutex_leave(db->mutex); - return SQLITE_BUSY; - } - assert( sqlite3SafetyCheckSickOrOk(db) ); + /* + ** Mark this database connection as a zombie. Then try to close it. + */ + db->magic = SQLITE_MAGIC_ZOMBIE; + sqlite3LeaveMutexAndCloseZombie(db); + return SQLITE_OK; +} + +/* +** Close the mutex on database connection db. +** +** Furthermore, if database connection db is a zombie (meaning that there +** has been a prior call to sqlite3_close(db)) and every sqlite3_stmt +** has now been finalized and every sqlite3_backup has finished, then +** free all resources. +*/ +void sqlite3LeaveMutexAndCloseZombie(sqlite3 *db){ + HashElem *i; /* Hash table iterator */ + int j; + + assert( sqlite3_mutex_held(db->mutex) ); + + /* If there are outstanding sqlite3_stmt or sqlite3_backup objects + ** or if the connection has not yet been closed by sqlite3_close, then + ** just leave the mutex and return. + */ + if( db->pVdbe || db->magic!=SQLITE_MAGIC_ZOMBIE ){ + sqlite3_mutex_leave(db->mutex); + return; + } for(j=0; jnDb; j++){ Btree *pBt = db->aDb[j].pBt; if( pBt && sqlite3BtreeIsInBackup(pBt) ){ - sqlite3Error(db, SQLITE_BUSY, - "unable to close due to unfinished backup operation"); sqlite3_mutex_leave(db->mutex); - return SQLITE_BUSY; + return; } } + /* If we reach this point, it means that the database connection has + ** closed all sqlite3_stmt and sqlite3_backup objects and has been + ** pased to sqlite3_close (meaning that it is a zombie). Therefore, + ** go ahead and free all resources. + */ + /* Free any outstanding Savepoint structures. */ sqlite3CloseSavepoints(db); @@ -845,7 +869,6 @@ int sqlite3_close(sqlite3 *db){ sqlite3_free(db->lookaside.pStart); } sqlite3_free(db); - return SQLITE_OK; } /* diff --git a/src/sqlite.h.in b/src/sqlite.h.in index e96c196802..ce07c386f6 100644 --- a/src/sqlite.h.in +++ b/src/sqlite.h.in @@ -265,12 +265,15 @@ typedef sqlite_uint64 sqlite3_uint64; ** ^Calls to sqlite3_close() return SQLITE_OK if the [sqlite3] object is ** successfully destroyed and all associated resources are deallocated. ** -** Applications must [sqlite3_finalize | finalize] all [prepared statements] -** and [sqlite3_blob_close | close] all [BLOB handles] associated with -** the [sqlite3] object prior to attempting to close the object. ^If +** Applications should [sqlite3_finalize | finalize] all [prepared statements], +** [sqlite3_blob_close | close] all [BLOB handles], and +** [sqlite3_backup_finish | finish] all [sqlite3_backup] objects associated +** with the [sqlite3] object prior to attempting to close the object. ^If ** sqlite3_close() is called on a [database connection] that still has -** outstanding [prepared statements] or [BLOB handles], then it returns -** SQLITE_BUSY. +** outstanding [prepared statements], [BLOB handles], and/or +** [sqlite3_backup] objects then it returns SQLITE_OK but the deallocation +** of resources is deferred until all [prepared statements], [BLOB handles], +** and [sqlite3_backup] objects are also destroyed. ** ** ^If [sqlite3_close()] is invoked while a transaction is open, ** the transaction is automatically rolled back. diff --git a/src/sqliteInt.h b/src/sqliteInt.h index 85dabb0b39..9732c8f226 100644 --- a/src/sqliteInt.h +++ b/src/sqliteInt.h @@ -974,6 +974,7 @@ struct sqlite3 { #define SQLITE_MAGIC_SICK 0x4b771290 /* Error and awaiting close */ #define SQLITE_MAGIC_BUSY 0xf03b7906 /* Database currently in use */ #define SQLITE_MAGIC_ERROR 0xb5357930 /* An SQLITE_MISUSE error occurred */ +#define SQLITE_MAGIC_ZOMBIE 0x64cffc7f /* Close with last statement close */ /* ** Each SQL function is defined by an instance of the following @@ -2833,6 +2834,7 @@ void sqlite3CommitTransaction(Parse*); void sqlite3RollbackTransaction(Parse*); void sqlite3Savepoint(Parse*, int, Token*); void sqlite3CloseSavepoints(sqlite3 *); +void sqlite3LeaveMutexAndCloseZombie(sqlite3*); int sqlite3ExprIsConstant(Expr*); int sqlite3ExprIsConstantNotJoin(Expr*); int sqlite3ExprIsConstantOrFunction(Expr*); diff --git a/src/vdbeapi.c b/src/vdbeapi.c index 94db205e1f..e25acd9449 100644 --- a/src/vdbeapi.c +++ b/src/vdbeapi.c @@ -71,17 +71,12 @@ int sqlite3_finalize(sqlite3_stmt *pStmt){ }else{ Vdbe *v = (Vdbe*)pStmt; sqlite3 *db = v->db; -#if SQLITE_THREADSAFE - sqlite3_mutex *mutex; -#endif if( vdbeSafety(v) ) return SQLITE_MISUSE_BKPT; -#if SQLITE_THREADSAFE - mutex = v->db->mutex; -#endif - sqlite3_mutex_enter(mutex); + sqlite3_mutex_enter(db->mutex); rc = sqlite3VdbeFinalize(v); + if( (rc&0xff)==SQLITE_MISUSE ) rc = SQLITE_OK; rc = sqlite3ApiExit(db, rc); - sqlite3_mutex_leave(mutex); + sqlite3LeaveMutexAndCloseZombie(db); } return rc; } diff --git a/src/vdbeaux.c b/src/vdbeaux.c index caa2bf6700..3ccf711619 100644 --- a/src/vdbeaux.c +++ b/src/vdbeaux.c @@ -2469,6 +2469,7 @@ void sqlite3VdbeDelete(Vdbe *p){ if( NEVER(p==0) ) return; db = p->db; + assert( sqlite3_mutex_held(db->mutex) ); if( p->pPrev ){ p->pPrev->pNext = p->pNext; }else{ diff --git a/test/backup.test b/test/backup.test index 4d7213c15c..bb517727a2 100644 --- a/test/backup.test +++ b/test/backup.test @@ -419,11 +419,8 @@ do_test backup-4.3.1 { } {B} do_test backup-4.3.2 { db2 cache flush - sqlite3_close db2 -} {SQLITE_BUSY} -do_test backup-4.3.3 { - sqlite3_errmsg db2 -} {unable to close due to unfinished backup operation} + db2 close ;# close will be deferred until the backup finishes +} {} do_test backup-4.3.4 { B step 50 } {SQLITE_DONE} @@ -436,7 +433,7 @@ do_test backup-4.4.1 { list $rc [sqlite3_errcode db] [sqlite3_errmsg db] } {1 SQLITE_ERROR {source and destination must be distinct}} db close -db2 close + do_test backup-4.5.1 { catch { forcedelete test.db } diff --git a/test/capi3.test b/test/capi3.test index d9106267c5..81cf41db0a 100644 --- a/test/capi3.test +++ b/test/capi3.test @@ -647,20 +647,14 @@ do_test capi3-6.0 { } {0} do_test capi3-6.1 { db cache flush - sqlite3_close $DB -} {SQLITE_BUSY} + db close +} {} do_test capi3-6.2 { sqlite3_step $STMT } {SQLITE_ERROR} -#check_data $STMT capi3-6.3 {INTEGER} {1} {1.0} {1} do_test capi3-6.3 { sqlite3_finalize $STMT } {SQLITE_SCHEMA} -do_test capi3-6.4-misuse { - db cache flush - sqlite3_close $DB -} {SQLITE_OK} -db close # This procedure sets the value of the file-format in file 'test.db' # to $newval. Also, the schema cookie is incremented. diff --git a/test/capi3c.test b/test/capi3c.test index 4092091894..3074ce09fc 100644 --- a/test/capi3c.test +++ b/test/capi3c.test @@ -618,22 +618,14 @@ do_test capi3c-6.0 { } {0} do_test capi3c-6.1 { db cache flush - sqlite3_close $DB -} {SQLITE_BUSY} -do_test capi3c-6.2 { + db close; # close deferred +} {} +do_test capi3c-6.2-misuse { sqlite3_step $STMT -} {SQLITE_ROW} -check_data $STMT capi3c-6.3 {INTEGER} {1} {1.0} {1} +} {SQLITE_MISUSE} do_test capi3c-6.3 { sqlite3_finalize $STMT } {SQLITE_OK} -do_test capi3c-6.4 { - db cache flush - sqlite3_close $DB -} {SQLITE_OK} -do_test capi3c-6.99-misuse { - db close -} {} # This procedure sets the value of the file-format in file 'test.db' # to $newval. Also, the schema cookie is incremented. diff --git a/test/misuse.test b/test/misuse.test index 71ee0118c8..62d387e3c6 100644 --- a/test/misuse.test +++ b/test/misuse.test @@ -170,7 +170,7 @@ do_test misuse-4.3 { } } msg] lappend v $msg $r -} {0 {} SQLITE_BUSY} +} {0 {} SQLITE_OK} do_test misuse-4.4 { # Flush the TCL statement cache here, otherwise the sqlite3_close() will # fail because there are still un-finalized() VDBEs.