From 94b30733f7d225b743389bf6722bf99f1430be97 Mon Sep 17 00:00:00 2001 From: danielk1977 Date: Thu, 2 Jul 2009 17:21:57 +0000 Subject: [PATCH] When a b-tree transaction is committed when there are open cursors, downgrade shared-cache write-locks to read-locks instead of relinquishing all locks. Fix for #3942. (CVS 6837) FossilOrigin-Name: 611e704fdf90a3d3932ca1cbab4be7e282bf1ddf --- manifest | 15 +++---- manifest.uuid | 2 +- src/btree.c | 97 ++++++++++++++++++++++++++++---------------- src/prepare.c | 6 +-- test/sharedlock.test | 55 +++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 48 deletions(-) create mode 100644 test/sharedlock.test diff --git a/manifest b/manifest index 3f6ef98dc4..11d4f01e8c 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Cause\sopening\sa\stransaction\son\sa\ssharable\sb-tree\smodule\sautomatically\sobtain\sa\sread-lock\son\spage\s1.\sThis\smeans\sthere\sis\sno\sway\sfor\ssqlite3BtreeGetMeta()\sto\sfail.\s(CVS\s6836) -D 2009-07-02T07:47:33 +C When\sa\sb-tree\stransaction\sis\scommitted\swhen\sthere\sare\sopen\scursors,\sdowngrade\sshared-cache\swrite-locks\sto\sread-locks\sinstead\sof\srelinquishing\sall\slocks.\sFix\sfor\s#3942.\s(CVS\s6837) +D 2009-07-02T17:21:58 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in 8b8fb7823264331210cddf103831816c286ba446 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -106,7 +106,7 @@ F src/auth.c 98db07c2088455797678eb1031f42d4d94d18a71 F src/backup.c 97a3859d8585eb4fcb1e81a795cf4b3fdd82f30f F src/bitvec.c 0ef0651714728055d43de7a4cdd95e703fac0119 F src/btmutex.c 9b899c0d8df3bd68f527b0afe03088321b696d3c -F src/btree.c b739b9659c07d2b9d20f0e8f51ae0cc7f995e039 +F src/btree.c 8a3f74aeea07833a7dd62ccd60c5486a17b2044c F src/btree.h 8cae6364735a5cb2d577ddb23fa6d0e655a4b931 F src/btreeInt.h b31e5ac04181c7e2892c33ab06228c551df6233c F src/build.c 867028ee9f63f7bc8eb8d4a720bb98cf9b9a12b4 @@ -153,7 +153,7 @@ F src/pcache.c 395f752a13574120bd7513a400ba02a265aaa76d F src/pcache.h 9b927ccc5a538e31b4c3bc7eec4f976db42a1324 F src/pcache1.c 97e7e8e6e34026fb43b47d08532b0c02e959c26c F src/pragma.c 9eb44ac1d3dc1ac3ea4f444abe1a10ae8acaa16c -F src/prepare.c e8d8996900d4b7b183622dd2fe2286e1ff0ad37f +F src/prepare.c 1f658bc796f2be51b898dc6cc913bca1911ca839 F src/printf.c 508a1c59433353552b6553cba175eaa7331f8fc1 F src/random.c 676b9d7ac820fe81e6fb2394ac8c10cff7f38628 F src/resolve.c 4a61d03e49b15440878096e6030863fc628828f0 @@ -542,6 +542,7 @@ F test/shared4.test d0fadacb50bb6981b2fb9dc6d1da30fa1edddf83 F test/shared6.test 990d2584b5db28e6e1f24742c711b26e59757b67 F test/shared7.test 8114027cb5e8c376e467115703d46e5ac4e77739 F test/shared_err.test 91e26ec4f3fbe07951967955585137e2f18993de +F test/sharedlock.test ffa0a3c4ac192145b310f1254f8afca4d553eabf F test/shortread1.test bb591ef20f0fd9ed26d0d12e80eee6d7ac8897a3 F test/sidedelete.test f0ad71abe6233e3b153100f3b8d679b19a488329 F test/soak.test d9d0a5e5c0157115c9a17f526f12691fe146768d @@ -738,7 +739,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl 672f81d693a03f80f5ae60bfefacd8a349e76746 -P 8f0591ae98c2125a4922933496f4412aee8ab86e -R 2a84fbc1846890230c75f17318268fe4 +P e3c055f167f895ae45858de9d9d8a264df2f36b6 +R acfaabd53533aef24e7f2e29a02278d4 U danielk1977 -Z 37d78b0a7f7b710a92735d2117f44c5f +Z 764555e312eb1c1b4aebbcabfc132eb3 diff --git a/manifest.uuid b/manifest.uuid index d2ed7911ab..e75dc84e4c 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -e3c055f167f895ae45858de9d9d8a264df2f36b6 \ No newline at end of file +611e704fdf90a3d3932ca1cbab4be7e282bf1ddf \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index 8b57837b28..7aafceaaf6 100644 --- a/src/btree.c +++ b/src/btree.c @@ -9,7 +9,7 @@ ** May you share freely, never taking more than you give. ** ************************************************************************* -** $Id: btree.c,v 1.648 2009/07/02 07:47:33 danielk1977 Exp $ +** $Id: btree.c,v 1.649 2009/07/02 17:21:58 danielk1977 Exp $ ** ** This file implements a external (disk-based) database using BTrees. ** See the header comment on "btreeInt.h" for additional information. @@ -81,6 +81,7 @@ int sqlite3_enable_shared_cache(int enable){ #define querySharedCacheTableLock(a,b,c) SQLITE_OK #define setSharedCacheTableLock(a,b,c) SQLITE_OK #define clearAllSharedCacheTableLocks(a) + #define downgradeAllSharedCacheTableLocks(a) #define hasSharedCacheTableLock(a,b,c,d) 1 #define hasReadConflicts(a, b) 0 #endif @@ -393,6 +394,21 @@ static void clearAllSharedCacheTableLocks(Btree *p){ pBt->isPending = 0; } } + +static void downgradeAllSharedCacheTableLocks(Btree *p){ + BtShared *pBt = p->pBt; + if( pBt->pWriter==p ){ + BtLock *pLock; + pBt->pWriter = 0; + pBt->isExclusive = 0; + pBt->isPending = 0; + for(pLock=pBt->pLock; pLock; pLock=pLock->pNext){ + assert( pLock->eLock==READ_LOCK || pLock->pBtree==p ); + pLock->eLock = READ_LOCK; + } + } +} + #endif /* SQLITE_OMIT_SHARED_CACHE */ static void releasePage(MemPage *pPage); /* Forward reference */ @@ -2901,6 +2917,48 @@ int sqlite3BtreeCommitPhaseOne(Btree *p, const char *zMaster){ return rc; } +/* +** This function is called from both BtreeCommitPhaseTwo() and BtreeRollback() +** at the conclusion of a transaction. +*/ +static void btreeEndTransaction(Btree *p){ + BtShared *pBt = p->pBt; + BtCursor *pCsr; + assert( sqlite3BtreeHoldsMutex(p) ); + + /* Search for a cursor held open by this b-tree connection. If one exists, + ** then the transaction will be downgraded to a read-only transaction + ** instead of actually concluded. A subsequent call to CommitPhaseTwo() + ** or Rollback() will finish the transaction and unlock the database. */ + for(pCsr=pBt->pCursor; pCsr && pCsr->pBtree!=p; pCsr=pCsr->pNext); + assert( pCsr==0 || p->inTrans>TRANS_NONE ); + + btreeClearHasContent(pBt); + if( pCsr ){ + downgradeAllSharedCacheTableLocks(p); + p->inTrans = TRANS_READ; + }else{ + /* If the handle had any kind of transaction open, decrement the + ** transaction count of the shared btree. If the transaction count + ** reaches 0, set the shared state to TRANS_NONE. The unlockBtreeIfUnused() + ** call below will unlock the pager. */ + if( p->inTrans!=TRANS_NONE ){ + clearAllSharedCacheTableLocks(p); + pBt->nTransaction--; + if( 0==pBt->nTransaction ){ + pBt->inTransaction = TRANS_NONE; + } + } + + /* Set the current transaction state to TRANS_NONE and unlock the + ** pager if this call closed the only read or write transaction. */ + p->inTrans = TRANS_NONE; + unlockBtreeIfUnused(pBt); + } + + btreeIntegrity(p); +} + /* ** Commit the transaction currently in progress. ** @@ -2937,27 +2995,7 @@ int sqlite3BtreeCommitPhaseTwo(Btree *p){ pBt->inTransaction = TRANS_READ; } - /* If the handle has any kind of transaction open, decrement the transaction - ** count of the shared btree. If the transaction count reaches 0, set - ** the shared state to TRANS_NONE. The unlockBtreeIfUnused() call below - ** will unlock the pager. - */ - if( p->inTrans!=TRANS_NONE ){ - clearAllSharedCacheTableLocks(p); - pBt->nTransaction--; - if( 0==pBt->nTransaction ){ - pBt->inTransaction = TRANS_NONE; - } - } - - /* Set the current transaction state to TRANS_NONE and unlock - ** the pager if this call closed the only read or write transaction. - */ - btreeClearHasContent(pBt); - p->inTrans = TRANS_NONE; - unlockBtreeIfUnused(pBt); - - btreeIntegrity(p); + btreeEndTransaction(p); sqlite3BtreeLeave(p); return SQLITE_OK; } @@ -3079,20 +3117,7 @@ int sqlite3BtreeRollback(Btree *p){ pBt->inTransaction = TRANS_READ; } - if( p->inTrans!=TRANS_NONE ){ - clearAllSharedCacheTableLocks(p); - assert( pBt->nTransaction>0 ); - pBt->nTransaction--; - if( 0==pBt->nTransaction ){ - pBt->inTransaction = TRANS_NONE; - } - } - - btreeClearHasContent(pBt); - p->inTrans = TRANS_NONE; - unlockBtreeIfUnused(pBt); - - btreeIntegrity(p); + btreeEndTransaction(p); sqlite3BtreeLeave(p); return rc; } diff --git a/src/prepare.c b/src/prepare.c index 792340a800..f68b802b39 100644 --- a/src/prepare.c +++ b/src/prepare.c @@ -13,7 +13,7 @@ ** interface, and routines that contribute to loading the database schema ** from disk. ** -** $Id: prepare.c,v 1.126 2009/07/02 07:47:33 danielk1977 Exp $ +** $Id: prepare.c,v 1.127 2009/07/02 17:21:58 danielk1977 Exp $ */ #include "sqliteInt.h" @@ -136,7 +136,7 @@ static int sqlite3InitOne(sqlite3 *db, int iDb, char **pzErrMsg){ InitData initData; char const *zMasterSchema; char const *zMasterName = SCHEMA_TABLE(iDb); - int openedTransaction; + int openedTransaction = 0; /* ** The master database table has a structure like this @@ -222,8 +222,6 @@ static int sqlite3InitOne(sqlite3 *db, int iDb, char **pzErrMsg){ goto initone_error_out; } openedTransaction = 1; - }else{ - openedTransaction = 0; } /* Get the database meta information. diff --git a/test/sharedlock.test b/test/sharedlock.test new file mode 100644 index 0000000000..1e78eeafdf --- /dev/null +++ b/test/sharedlock.test @@ -0,0 +1,55 @@ +# 2009 July 2 +# +# 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. +# +#*********************************************************************** +# +# $Id: sharedlock.test,v 1.1 2009/07/02 17:21:58 danielk1977 Exp $ + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +db close + +ifcapable !shared_cache { + finish_test + return +} + +set ::enable_shared_cache [sqlite3_enable_shared_cache 1] +sqlite3 db test.db +sqlite3 db2 test.db + +do_test sharedlock-1.1 { + execsql { + CREATE TABLE t1(a, b); + INSERT INTO t1 VALUES(1, 'one'); + INSERT INTO t1 VALUES(2, 'two'); + } +} {} + +do_test sharedlock-1.2 { + set res [list] + db eval { SELECT * FROM t1 ORDER BY rowid } { + lappend res $a $b + if {$a == 1} { catch { db eval "INSERT INTO t1 VALUES(3, 'three')" } } + + # This should fail. Connection [db] has a read-lock on t1, which should + # prevent connection [db2] from obtaining the write-lock it needs to + # modify t1. At one point there was a bug causing the previous INSERT + # to drop the read-lock belonging to [db]. + if {$a == 2} { catch { db2 eval "INSERT INTO t1 VALUES(4, 'four')" } } + } + set res +} {1 one 2 two 3 three} + +db close +db2 close + +sqlite3_enable_shared_cache $::enable_shared_cache +finish_test +