From 2a50ff030966188d49186307157f416567d82a96 Mon Sep 17 00:00:00 2001 From: danielk1977 Date: Fri, 10 Apr 2009 09:47:06 +0000 Subject: [PATCH] Always set BtShared.db when entering the BtShared mutex. Ticket #3793. (CVS 6480) FossilOrigin-Name: ed6620ba589ddbb6ca86f42a7652e3b019195647 --- manifest | 17 ++++---- manifest.uuid | 2 +- src/btmutex.c | 79 +++++++++++++++++++++------------ src/btree.c | 19 +------- test/tkt3793.test | 109 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 173 insertions(+), 53 deletions(-) create mode 100644 test/tkt3793.test diff --git a/manifest b/manifest index 75150435e8..ac6831495f 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Force\s8-byte\salignment\sof\ssqlite3_value\sobjects\sin\sthe\nsqlite3VdbeUnpackRecord()\sprimitive.\s\sTicket\s#3777.\s(CVS\s6479) -D 2009-04-10T00:56:28 +C Always\sset\sBtShared.db\swhen\sentering\sthe\sBtShared\smutex.\sTicket\s#3793.\s(CVS\s6480) +D 2009-04-10T09:47:07 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in 583e87706abc3026960ed759aff6371faf84c211 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -102,8 +102,8 @@ F src/attach.c af80fa85d391ad302c148c4e2524a2cebec64cb2 F src/auth.c c8b2ab5c8bad4bd90ed7c294694f48269162c627 F src/backup.c 0082d0e5a63f04e88faee0dff0a7d63d3e92a78d F src/bitvec.c ef370407e03440b0852d05024fb016b14a471d3d -F src/btmutex.c 341502bc496dc0840dcb00cde65680fb0e85c3ab -F src/btree.c 02c902db5527fc20b74a9ffdf1fc296ee3437964 +F src/btmutex.c 527b63e275d132564710dec47ddab52a0f2d7f4e +F src/btree.c 8331febf3769cdac2e0cde463be4ed901406b783 F src/btree.h 8007018c1753944790c39610280894ab280210b8 F src/btreeInt.h df64030d632f8c8ac217ed52e8b6b3eacacb33a5 F src/build.c 2882f22078db1c3f887b1aca77ff460cf9461c62 @@ -637,6 +637,7 @@ F test/tkt3761.test b95ea9c98f21cf91325f18a984887e62caceab33 F test/tkt3762.test 2a9f3b03df44ec49ec0cfa8d5da6574c2a7853df F test/tkt3773.test 430b06567ce40285dfd2c4834a2a61816403efeb F test/tkt3791.test a6624b9a80b216a26cf473607f42f3e51898c267 +F test/tkt3793.test 3aa2efe55bc31fc9459618feea2016ea9a52b2af F test/tokenize.test ce430a7aed48fc98301611429595883fdfcab5d7 F test/trace.test 19ffbc09885c3321d56358a5738feae8587fb377 F test/trans.test 8b79967a7e085289ec64890c6fdf9d089e1b4a5f @@ -716,7 +717,7 @@ F tool/speedtest16.c c8a9c793df96db7e4933f0852abb7a03d48f2e81 F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e -P 9a09a47495d498a3372ead0eef5e3642a3ff30c2 -R 5607ab4efa85fc4682dba26b62f8935a -U drh -Z d8359058aa4314f29eca77f3a23b5b1c +P 2cc68272b1f70701268075cfa82fa64bb2a8179d +R 0f5ed1cbc4ec520cfd83afb78d3917f3 +U danielk1977 +Z 46ae6a8835e1cba4786ac026de3d9fbb diff --git a/manifest.uuid b/manifest.uuid index 3a72b4506d..154252be0e 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -2cc68272b1f70701268075cfa82fa64bb2a8179d \ No newline at end of file +ed6620ba589ddbb6ca86f42a7652e3b019195647 \ No newline at end of file diff --git a/src/btmutex.c b/src/btmutex.c index 4d2f767995..06cbed8d9c 100644 --- a/src/btmutex.c +++ b/src/btmutex.c @@ -10,7 +10,7 @@ ** ************************************************************************* ** -** $Id: btmutex.c,v 1.13 2009/03/05 04:20:32 shane Exp $ +** $Id: btmutex.c,v 1.14 2009/04/10 09:47:07 danielk1977 Exp $ ** ** This file contains code used to implement mutexes on Btree objects. ** This code really belongs in btree.c. But btree.c is getting too @@ -20,6 +20,34 @@ #include "btreeInt.h" #if SQLITE_THREADSAFE && !defined(SQLITE_OMIT_SHARED_CACHE) +/* +** Obtain the BtShared mutex associated with B-Tree handle p. Also, +** set BtShared.db to the database handle associated with p and the +** p->locked boolean to true. +*/ +static void lockBtreeMutex(Btree *p){ + assert( p->locked==0 ); + assert( sqlite3_mutex_notheld(p->pBt->mutex) ); + assert( sqlite3_mutex_held(p->db->mutex) ); + + sqlite3_mutex_enter(p->pBt->mutex); + p->pBt->db = p->db; + p->locked = 1; +} + +/* +** Release the BtShared mutex associated with B-Tree handle p and +** clear the p->locked boolean. +*/ +static void unlockBtreeMutex(Btree *p){ + assert( p->locked==1 ); + assert( sqlite3_mutex_held(p->pBt->mutex) ); + assert( sqlite3_mutex_held(p->db->mutex) ); + assert( p->db==p->pBt->db ); + + sqlite3_mutex_leave(p->pBt->mutex); + p->locked = 0; +} /* ** Enter a mutex on the given BTree object. @@ -57,6 +85,10 @@ void sqlite3BtreeEnter(Btree *p){ /* We should already hold a lock on the database connection */ assert( sqlite3_mutex_held(p->db->mutex) ); + /* Unless the database is sharable and unlocked, then BtShared.db + ** should already be set correctly. */ + assert( (p->locked==0 && p->sharable) || p->pBt->db==p->db ); + if( !p->sharable ) return; p->wantToLock++; if( p->locked ) return; @@ -66,6 +98,7 @@ void sqlite3BtreeEnter(Btree *p){ ** procedure that follows. Just be sure not to block. */ if( sqlite3_mutex_try(p->pBt->mutex)==SQLITE_OK ){ + p->pBt->db = p->db; p->locked = 1; return; } @@ -80,16 +113,13 @@ void sqlite3BtreeEnter(Btree *p){ assert( pLater->pNext==0 || pLater->pNext->pBt>pLater->pBt ); assert( !pLater->locked || pLater->wantToLock>0 ); if( pLater->locked ){ - sqlite3_mutex_leave(pLater->pBt->mutex); - pLater->locked = 0; + unlockBtreeMutex(pLater); } } - sqlite3_mutex_enter(p->pBt->mutex); - p->locked = 1; + lockBtreeMutex(p); for(pLater=p->pNext; pLater; pLater=pLater->pNext){ if( pLater->wantToLock ){ - sqlite3_mutex_enter(pLater->pBt->mutex); - pLater->locked = 1; + lockBtreeMutex(pLater); } } } @@ -102,25 +132,25 @@ void sqlite3BtreeLeave(Btree *p){ assert( p->wantToLock>0 ); p->wantToLock--; if( p->wantToLock==0 ){ - assert( p->locked ); - sqlite3_mutex_leave(p->pBt->mutex); - p->locked = 0; + unlockBtreeMutex(p); } } } #ifndef NDEBUG /* -** Return true if the BtShared mutex is held on the btree. -** -** This routine makes no determination one way or another if the -** database connection mutex is held. +** Return true if the BtShared mutex is held on the btree, or if the +** B-Tree is not marked as sharable. ** ** This routine is used only from within assert() statements. */ int sqlite3BtreeHoldsMutex(Btree *p){ - return (p->sharable==0 || - (p->locked && p->wantToLock && sqlite3_mutex_held(p->pBt->mutex))); + assert( p->sharable==0 || p->locked==0 || p->wantToLock>0 ); + assert( p->sharable==0 || p->locked==0 || p->db==p->pBt->db ); + assert( p->sharable==0 || p->locked==0 || sqlite3_mutex_held(p->pBt->mutex) ); + assert( p->sharable==0 || p->locked==0 || sqlite3_mutex_held(p->db->mutex) ); + + return (p->sharable==0 || p->locked); } #endif @@ -160,6 +190,7 @@ void sqlite3BtreeEnterAll(sqlite3 *db){ assert( sqlite3_mutex_held(db->mutex) ); for(i=0; inDb; i++){ p = db->aDb[i].pBt; + assert( !p || (p->locked==0 && p->sharable) || p->pBt->db==p->db ); if( p && p->sharable ){ p->wantToLock++; if( !p->locked ){ @@ -168,13 +199,11 @@ void sqlite3BtreeEnterAll(sqlite3 *db){ while( p->locked && p->pNext ) p = p->pNext; for(pLater = p->pNext; pLater; pLater=pLater->pNext){ if( pLater->locked ){ - sqlite3_mutex_leave(pLater->pBt->mutex); - pLater->locked = 0; + unlockBtreeMutex(pLater); } } while( p ){ - sqlite3_mutex_enter(p->pBt->mutex); - p->locked++; + lockBtreeMutex(p); p = p->pNext; } } @@ -191,9 +220,7 @@ void sqlite3BtreeLeaveAll(sqlite3 *db){ assert( p->wantToLock>0 ); p->wantToLock--; if( p->wantToLock==0 ){ - assert( p->locked ); - sqlite3_mutex_leave(p->pBt->mutex); - p->locked = 0; + unlockBtreeMutex(p); } } } @@ -282,8 +309,7 @@ void sqlite3BtreeMutexArrayEnter(BtreeMutexArray *pArray){ p->wantToLock++; if( !p->locked && p->sharable ){ - sqlite3_mutex_enter(p->pBt->mutex); - p->locked = 1; + lockBtreeMutex(p); } } } @@ -305,8 +331,7 @@ void sqlite3BtreeMutexArrayLeave(BtreeMutexArray *pArray){ p->wantToLock--; if( p->wantToLock==0 && p->locked ){ - sqlite3_mutex_leave(p->pBt->mutex); - p->locked = 0; + unlockBtreeMutex(p); } } } diff --git a/src/btree.c b/src/btree.c index 61fed3dca3..e5c25f8090 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.593 2009/04/10 00:56:28 drh Exp $ +** $Id: btree.c,v 1.594 2009/04/10 09:47:07 danielk1977 Exp $ ** ** This file implements a external (disk-based) database using BTrees. ** See the header comment on "btreeInt.h" for additional information. @@ -1470,6 +1470,7 @@ int sqlite3BtreeOpen( if( rc!=SQLITE_OK ){ goto btree_open_out; } + pBt->db = db; sqlite3PagerSetBusyhandler(pBt->pPager, btreeInvokeBusyHandler, pBt); p->pBt = pBt; @@ -1647,7 +1648,6 @@ int sqlite3BtreeClose(Btree *p){ /* Close all cursors opened via this handle. */ assert( sqlite3_mutex_held(p->db->mutex) ); sqlite3BtreeEnter(p); - pBt->db = p->db; pCur = pBt->pCursor; while( pCur ){ BtCursor *pTmp = pCur; @@ -2126,7 +2126,6 @@ int sqlite3BtreeBeginTrans(Btree *p, int wrflag){ int rc = SQLITE_OK; sqlite3BtreeEnter(p); - pBt->db = p->db; btreeIntegrity(p); /* If the btree is already in a write-transaction, or it @@ -2545,7 +2544,6 @@ int sqlite3BtreeIncrVacuum(Btree *p){ BtShared *pBt = p->pBt; sqlite3BtreeEnter(p); - pBt->db = p->db; assert( pBt->inTransaction==TRANS_WRITE && p->inTrans==TRANS_WRITE ); if( !pBt->autoVacuum ){ rc = SQLITE_DONE; @@ -2652,7 +2650,6 @@ int sqlite3BtreeCommitPhaseOne(Btree *p, const char *zMaster){ if( p->inTrans==TRANS_WRITE ){ BtShared *pBt = p->pBt; sqlite3BtreeEnter(p); - pBt->db = p->db; #ifndef SQLITE_OMIT_AUTOVACUUM if( pBt->autoVacuum ){ rc = autoVacuumCommit(pBt); @@ -2686,7 +2683,6 @@ int sqlite3BtreeCommitPhaseTwo(Btree *p){ BtShared *pBt = p->pBt; sqlite3BtreeEnter(p); - pBt->db = p->db; btreeIntegrity(p); /* If the handle has a write-transaction open, commit the shared-btrees @@ -2812,7 +2808,6 @@ int sqlite3BtreeRollback(Btree *p){ MemPage *pPage1; sqlite3BtreeEnter(p); - pBt->db = p->db; rc = saveAllCursors(pBt, 0, 0); #ifndef SQLITE_OMIT_SHARED_CACHE if( rc!=SQLITE_OK ){ @@ -2887,7 +2882,6 @@ int sqlite3BtreeBeginStmt(Btree *p, int iStatement){ int rc; BtShared *pBt = p->pBt; sqlite3BtreeEnter(p); - pBt->db = p->db; assert( p->inTrans==TRANS_WRITE ); assert( pBt->readOnly==0 ); assert( iStatement>0 ); @@ -2926,7 +2920,6 @@ int sqlite3BtreeSavepoint(Btree *p, int op, int iSavepoint){ assert( op==SAVEPOINT_RELEASE || op==SAVEPOINT_ROLLBACK ); assert( iSavepoint>=0 || (iSavepoint==-1 && op==SAVEPOINT_ROLLBACK) ); sqlite3BtreeEnter(p); - pBt->db = p->db; rc = sqlite3PagerSavepoint(pBt->pPager, op, iSavepoint); if( rc==SQLITE_OK ){ rc = newDatabase(pBt); @@ -3043,7 +3036,6 @@ int sqlite3BtreeCursor( ){ int rc; sqlite3BtreeEnter(p); - p->pBt->db = p->db; rc = btreeCursor(p, iTable, wrFlag, pKeyInfo, pCur); sqlite3BtreeLeave(p); return rc; @@ -3101,7 +3093,6 @@ int sqlite3BtreeCloseCursor(BtCursor *pCur){ int i; BtShared *pBt = pCur->pBt; sqlite3BtreeEnter(pBtree); - pBt->db = pBtree->db; sqlite3BtreeClearCursor(pCur); if( pCur->pPrev ){ pCur->pPrev->pNext = pCur->pNext; @@ -6517,7 +6508,6 @@ static int btreeCreateTable(Btree *p, int *piTable, int flags){ int sqlite3BtreeCreateTable(Btree *p, int *piTable, int flags){ int rc; sqlite3BtreeEnter(p); - p->pBt->db = p->db; rc = btreeCreateTable(p, piTable, flags); sqlite3BtreeLeave(p); return rc; @@ -6589,7 +6579,6 @@ int sqlite3BtreeClearTable(Btree *p, int iTable, int *pnChange){ int rc; BtShared *pBt = p->pBt; sqlite3BtreeEnter(p); - pBt->db = p->db; assert( p->inTrans==TRANS_WRITE ); if( (rc = checkForReadConflicts(p, iTable, 0, 1))!=SQLITE_OK ){ /* nothing to do */ @@ -6731,7 +6720,6 @@ static int btreeDropTable(Btree *p, Pgno iTable, int *piMoved){ int sqlite3BtreeDropTable(Btree *p, int iTable, int *piMoved){ int rc; sqlite3BtreeEnter(p); - p->pBt->db = p->db; rc = btreeDropTable(p, iTable, piMoved); sqlite3BtreeLeave(p); return rc; @@ -6755,7 +6743,6 @@ int sqlite3BtreeGetMeta(Btree *p, int idx, u32 *pMeta){ BtShared *pBt = p->pBt; sqlite3BtreeEnter(p); - pBt->db = p->db; /* Reading a meta-data value requires a read-lock on page 1 (and hence ** the sqlite_master table. We grab this lock regardless of whether or @@ -6826,7 +6813,6 @@ int sqlite3BtreeUpdateMeta(Btree *p, int idx, u32 iMeta){ int rc; assert( idx>=1 && idx<=15 ); sqlite3BtreeEnter(p); - pBt->db = p->db; assert( p->inTrans==TRANS_WRITE ); assert( pBt->pPage1!=0 ); pP1 = pBt->pPage1->aData; @@ -7300,7 +7286,6 @@ char *sqlite3BtreeIntegrityCheck( char zErr[100]; sqlite3BtreeEnter(p); - pBt->db = p->db; nRef = sqlite3PagerRefcount(pBt->pPager); if( lockBtreeWithRetry(p)!=SQLITE_OK ){ *pnErr = 1; diff --git a/test/tkt3793.test b/test/tkt3793.test new file mode 100644 index 0000000000..5cba0a1511 --- /dev/null +++ b/test/tkt3793.test @@ -0,0 +1,109 @@ +# 2009 April 10 +# +# 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. +# +#*********************************************************************** +# This file implements regression tests for SQLite library. +# +# This file implements tests to verify that ticket #3793 has been +# fixed. +# +# $Id: tkt3793.test,v 1.1 2009/04/10 09:47:07 danielk1977 Exp $ + + +set testdir [file dirname $argv0] +source $testdir/tester.tcl + +ifcapable !shared_cache||!attach { + finish_test + return +} +set ::enable_shared_cache [sqlite3_enable_shared_cache 1] + +do_test tkt3793-1.1 { + sqlite3 db1 test.db + sqlite3 db2 test.db + execsql { + BEGIN; + CREATE TABLE t1(a, b); + CREATE TABLE t2(a PRIMARY KEY, b); + INSERT INTO t1 VALUES(randstr(50,50), randstr(50,50)); + INSERT INTO t1 SELECT randstr(50,50), randstr(50,50) FROM t1; + INSERT INTO t1 SELECT randstr(50,50), randstr(50,50) FROM t1; + INSERT INTO t1 SELECT randstr(50,50), randstr(50,50) FROM t1; + INSERT INTO t1 SELECT randstr(50,50), randstr(50,50) FROM t1; + INSERT INTO t1 SELECT randstr(50,50), randstr(50,50) FROM t1; + INSERT INTO t1 SELECT randstr(50,50), randstr(50,50) FROM t1; + INSERT INTO t1 SELECT randstr(50,50), randstr(50,50) FROM t1; + INSERT INTO t1 SELECT randstr(50,50), randstr(50,50) FROM t1; + INSERT INTO t1 SELECT randstr(50,50), randstr(50,50) FROM t1; + INSERT INTO t1 SELECT randstr(50,50), randstr(50,50) FROM t1; + INSERT INTO t2 SELECT * FROM t1; + COMMIT; + } +} {} + +proc busyhandler {db args} { set ::busyconnection $db ; return 1 } +db2 busy {busyhandler db2} +db1 busy {busyhandler db1} + +# Establish a read-lock on the database file using connection [db]. +# +do_test tkt3793-1.2 { + execsql { + BEGIN; + SELECT count(*) FROM t1; + } +} {1024} + +# Set the size of the cache shared by [db1] and [db2] to 10. Then update +# more than 10 pages of table t1. At this point the shared-cache will +# hold a RESERVED lock on the database file. Even though there are now +# more than 10 dirty pages in memory, it cannot upgrade to an EXCLUSIVE +# lock because of the read-lock held by [db]. +# +do_test tkt3793-1.3 { + execsql { + PRAGMA cache_size = 10; + BEGIN; + UPDATE t1 SET b = randstr(50,50); + } db1 +} {} + +set x 0 + +# Run one SELECT query on the shared-cache using [db1], then from within +# the callback run another via [db2]. Because of the large number of dirty +# pages within the cache, each time a new page is read from the database +# SQLite will attempt to upgrade to an EXCLUSIVE lock, and hence invoke +# the busy-handler. The tests here verify that the correct busy-handler +# function is invoked (the busy-handler associated with the database +# connection that called sqlite3_step()). When bug #3793 existed, sometimes +# the [db2] busy-handler was invoked from within the call to sqlite3_step() +# associated with [db1]. +# +# Note: Before the bug was fixed, if [db2] was opened with the "-fullmutex 1" +# option, then this test case would cause an assert() to fail. +# +set ::busyconnection db1 +db1 eval {SELECT * FROM t2 ORDER BY a LIMIT 20} { + do_test tkt3793-2.[incr x] { set ::busyconnection } db1 + set ::busyconnection db2 + + db2 eval { SELECT count(*) FROM t2 } + do_test tkt3793-2.[incr x] { set ::busyconnection } db2 + set ::busyconnection db1 +} + +do_test tkt3793-3 { + db1 close + db2 close +} {} + +sqlite3_enable_shared_cache $::enable_shared_cache +finish_test