From c74d0b1dfcfe74b6d1eef6c37a6029913d28c6d5 Mon Sep 17 00:00:00 2001 From: drh Date: Tue, 24 Feb 2009 16:18:05 +0000 Subject: [PATCH] Enhanced comments on table locking logic as it relates to preparing new statements. Added assert() and testcase() but no other changes to code. (CVS 6319) FossilOrigin-Name: 4a12f5b818b769d7518c942ff3dedf453dde698e --- manifest | 16 ++++++++-------- manifest.uuid | 2 +- src/btree.c | 18 ++++++++++-------- src/prepare.c | 28 +++++++++++++++++++++++++--- 4 files changed, 44 insertions(+), 20 deletions(-) diff --git a/manifest b/manifest index b1238d9919..cbc7d9d58b 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\stest\sfile\stest/count.test\sfor\stesting\s"SELECT\scount(*)"\sstatements.\sIt\sis\snot\sproperly\spopulated\syet.\s(CVS\s6318) -D 2009-02-24T10:48:28 +C Enhanced\scomments\son\stable\slocking\slogic\sas\sit\srelates\sto\spreparing\snew\nstatements.\s\sAdded\sassert()\sand\stestcase()\sbut\sno\sother\schanges\sto\scode.\s(CVS\s6319) +D 2009-02-24T16:18:05 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in d64baddbf55cdf33ff030e14da837324711a4ef7 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -103,7 +103,7 @@ F src/auth.c c8b2ab5c8bad4bd90ed7c294694f48269162c627 F src/backup.c 2d3f31148d7b086c5c72d9edcd04fc2751b0aa6e F src/bitvec.c 44f7059ac1f874d364b34af31b9617e52223ba75 F src/btmutex.c 63c5cc4ad5715690767ffcb741e185d7bc35ec1a -F src/btree.c e0178d6fb69c8f332f3fba96cfc0b08275ad5e76 +F src/btree.c 8b8697ab5e8ca9aad12221bb9abfee1aed7a5cd1 F src/btree.h 96a019c9f28da38e79940512d7800e419cd8c702 F src/btreeInt.h 0a4884e6152d7cae9c741e91b830064c19fd2c05 F src/build.c a1db48aec62c95049d783f231195812ff97ae268 @@ -149,7 +149,7 @@ F src/pcache.c fcf7738c83c4d3e9d45836b2334c8a368cc41274 F src/pcache.h 9b927ccc5a538e31b4c3bc7eec4f976db42a1324 F src/pcache1.c dabb8ab14827e090321f17150ce96fda172974e8 F src/pragma.c 22ed04836aab8ce134c53be1ca896f3ad20fabdb -F src/prepare.c d0bbb4b1a8b9c1db6d13788929839bb63764680e +F src/prepare.c 1ede93a00f1835a10b3d3aad968e8f8ecd2c98dd F src/printf.c 9866a9a9c4a90f6d4147407f373df3fd5d5f9b6f F src/random.c 676b9d7ac820fe81e6fb2394ac8c10cff7f38628 F src/resolve.c dea005135845acbbfae1f2968c0deb6b2e3d580c @@ -702,7 +702,7 @@ F tool/speedtest16.c c8a9c793df96db7e4933f0852abb7a03d48f2e81 F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e -P 0e7c369c23a8767b4d3e5cdd47c14716992fb71a -R 71f287412eb030dfd9b14d26e52b17fb -U danielk1977 -Z e2d5b34166a5d213f9445c572c8e416e +P a195d74ff9ce836447dba4da7edcc6f1cdae5574 +R 7311b1fa9b7b5c3d12ba433576f5510b +U drh +Z 99368cf9c56a6d99659a698dff53e252 diff --git a/manifest.uuid b/manifest.uuid index b237de885f..e181bcc7c0 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -a195d74ff9ce836447dba4da7edcc6f1cdae5574 \ No newline at end of file +4a12f5b818b769d7518c942ff3dedf453dde698e \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index 07169bed3d..9843acb0ab 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.567 2009/02/24 10:01:52 danielk1977 Exp $ +** $Id: btree.c,v 1.568 2009/02/24 16:18:05 drh Exp $ ** ** This file implements a external (disk-based) database using BTrees. ** See the header comment on "btreeInt.h" for additional information. @@ -113,8 +113,9 @@ static int queryTableLock(Btree *p, Pgno iTab, u8 eLock){ } /* This (along with lockTable()) is where the ReadUncommitted flag is - ** dealt with. If the caller is querying for a read-lock and the flag is - ** set, it is unconditionally granted - even if there are write-locks + ** dealt with. If the caller is querying for a read-lock on any table + ** other than the sqlite_master table (table 1) and if the ReadUncommitted + ** flag is set, then the lock granted even if there are write-locks ** on the table. If a write-lock is requested, the ReadUncommitted flag ** is not considered. ** @@ -122,9 +123,9 @@ static int queryTableLock(Btree *p, Pgno iTab, u8 eLock){ ** ReadUncommitted flag is set, no entry is added to the locks list ** (BtShared.pLock). ** - ** To summarize: If the ReadUncommitted flag is set, then read cursors do - ** not create or respect table locks. The locking procedure for a - ** write-cursor does not change. + ** To summarize: If the ReadUncommitted flag is set, then read cursors + ** on non-schema tables do not create or respect table locks. The locking + ** procedure for a write-cursor does not change. */ if( 0==(p->db->flags&SQLITE_ReadUncommitted) || @@ -167,8 +168,9 @@ static int lockTable(Btree *p, Pgno iTable, u8 eLock){ assert( SQLITE_OK==queryTableLock(p, iTable, eLock) ); - /* If the read-uncommitted flag is set and a read-lock is requested, - ** return early without adding an entry to the BtShared.pLock list. See + /* If the read-uncommitted flag is set and a read-lock is requested on + ** a non-schema table, then the lock is always granted. Return early + ** without adding an entry to the BtShared.pLock list. See ** comment in function queryTableLock() for more info on handling ** the ReadUncommitted flag. */ diff --git a/src/prepare.c b/src/prepare.c index e2dbaf52fa..fbd005cfb3 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.106 2009/02/19 14:39:25 danielk1977 Exp $ +** $Id: prepare.c,v 1.107 2009/02/24 16:18:05 drh Exp $ */ #include "sqliteInt.h" @@ -540,17 +540,39 @@ static int sqlite3Prepare( assert( !db->mallocFailed ); assert( sqlite3_mutex_held(db->mutex) ); - /* If any attached database schemas are locked, do not proceed with - ** compilation. Instead return SQLITE_LOCKED immediately. + /* Check to verify that it is possible to get a read lock on all + ** database schemas. The inability to get a read lock indicates that + ** some other database connection is holding a write-lock, which in + ** turn means that the other connection has made uncommitted changes + ** to the schema. + ** + ** Were we to proceed and prepare the statement against the uncommitted + ** schema changes and if those schema changes are subsequently rolled + ** back and different changes are made in their place, then when this + ** prepared statement goes to run the schema cookie would fail to detect + ** the schema change. Disaster would follow. + ** + ** This thread is currently holding mutexes on all Btrees (because + ** of the sqlite3BtreeEnterAll() in sqlite3LockAndPrepare()) so it + ** is not possible for another thread to start a new schema change + ** while this routine is running. Hence, we do not need to hold + ** locks on the schema, we just need to make sure nobody else is + ** holding them. + ** + ** Note that setting READ_UNCOMMITTED overrides most lock detection, + ** but it does *not* override schema lock detection, so this all still + ** works even if READ_UNCOMMITTED is set. */ for(i=0; inDb; i++) { Btree *pBt = db->aDb[i].pBt; if( pBt ){ + assert( sqlite3BtreeHoldsMutex(pBt) ); rc = sqlite3BtreeSchemaLocked(pBt); if( rc ){ const char *zDb = db->aDb[i].zName; sqlite3Error(db, SQLITE_LOCKED, "database schema is locked: %s", zDb); (void)sqlite3SafetyOff(db); + testcase( db->flags & SQLITE_ReadUncommitted ); return sqlite3ApiExit(db, SQLITE_LOCKED); } }