diff --git a/manifest b/manifest index 4963775d14..ce4e8a342d 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Do\snot\sinvoke\sthe\sbusy\scallback\swhen\strying\sto\spromote\sa\slock\sfrom\sSHARED\nto\sRESERVED.\s\sThis\savoids\sa\sdeadlock.\s(CVS\s1879) -D 2004-08-07T23:54:48 +C Attempting\sto\sopen\sa\slocked\stable\sfor\swriting\sshould\sfail\simmediately.\r\nTicket\s#842.\s(CVS\s1880) +D 2004-08-08T19:43:30 F Makefile.in 4a5e570a9e2d35b09c31b3cf01b78cea764ade4b F Makefile.linux-gcc a9e5a0d309fa7c38e7c14d3ecf7690879d3a5457 F README f1de682fbbd94899d50aca13d387d1b3fd3be2dd @@ -27,7 +27,7 @@ F sqlite3.def c139a6b17293d71759a7c301ad76707243ce5d54 F sqlite3.pc.in 985b9bf34192a549d7d370e0f0b6b34a4f61369a F src/attach.c 0bd4f11da6999665da30625665a4096ba7898de6 F src/auth.c 60db23b98bb94c8b0178180faaf49dc116674217 -F src/btree.c edf4ece708350dec7f28ebd4620c6d33afe6993a +F src/btree.c cfa5ed01838247be40c113cac528286675940801 F src/btree.h 94dfec0a1722d33359b23e7e310f2b64ffedf029 F src/build.c d1a2d7a99bb07a1ea4a019fcef6786546cb09f73 F src/date.c e1bb384a7856c18dce9cadb0afbe6934ba5ddb00 @@ -109,6 +109,7 @@ F test/crash.test 3ea432ce624369c04ba1a23a5288115e40f5daa2 F test/crashtest1.c 09c1c7d728ccf4feb9e481671e29dda5669bbcc2 F test/date.test a5cdaed88fe575f2d6f63ff605abb5abc1b7319c F test/delete.test ec0b455f2dcc0e189d96ee438438ba026c4e51d8 +F test/delete2.test 050a3a6e8ea0f83aed817d164b16af2a499fb452 F test/enc.test 2f5463af488d50aef60c6110bec6b21b5efba961 F test/enc2.test 7a60971a62748be6b607b4b4380eb4c5e151a6ec F test/enc3.test 2ae80b11adf5b2c171d2e17214dabd356b9672c1 @@ -241,7 +242,7 @@ F www/tclsqlite.tcl 06a86cba4d7fc88e2bcd633b57702d3d16abebb5 F www/vdbe.tcl 59288db1ac5c0616296b26dce071c36cb611dfe9 F www/version3.tcl 092a01f5ef430d2c4acc0ae558d74c4bb89638a0 F www/whentouse.tcl a8335bce47cc2fddb07f19052cb0cb4d9129a8e4 -P 863540be248d3079e1a997349be6c74199149511 -R 537860faa651d2512143eecc05e62317 +P d33771a303d9c20dd477b1a973024ff763203211 +R d49220a972e86ed5f2222c14670b9ccc U drh -Z a6e6f8dc54fe5cdf4e420d0a0243394b +Z 6bdb053e7f2204021eb022af1f2d8d7a diff --git a/manifest.uuid b/manifest.uuid index 428204a92c..81d0aaea63 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -d33771a303d9c20dd477b1a973024ff763203211 \ No newline at end of file +fc879a9b1d05ddb8f8c552c1d334597e41b29b27 \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index 19162598c1..3f9992caa1 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.180 2004/07/23 00:01:39 drh Exp $ +** $Id: btree.c,v 1.181 2004/08/08 19:43:30 drh Exp $ ** ** This file implements a external (disk-based) database using BTrees. ** For a detailed discussion of BTrees, refer to @@ -338,7 +338,6 @@ struct CellInfo { struct BtCursor { Btree *pBt; /* The Btree to which this cursor belongs */ BtCursor *pNext, *pPrev; /* Forms a linked list of all cursors */ - BtCursor *pShared; /* Loop of cursors with the same root page */ int (*xCompare)(void*,int,const void*,int,const void*); /* Key comp func */ void *pArg; /* First arg to xCompare() */ Pgno pgnoRoot; /* The root page of this tree */ @@ -1532,16 +1531,21 @@ int sqlite3BtreeCursor( BtCursor **ppCur /* Write new cursor here */ ){ int rc; - BtCursor *pCur, *pRing; + BtCursor *pCur; - if( pBt->readOnly && wrFlag ){ - *ppCur = 0; - return SQLITE_READONLY; + *ppCur = 0; + if( wrFlag ){ + static int checkReadLocks(Btree*,Pgno,BtCursor*); + if( pBt->readOnly ){ + return SQLITE_READONLY; + } + if( checkReadLocks(pBt, iTable, 0) ){ + return SQLITE_LOCKED; + } } if( pBt->pPage1==0 ){ rc = lockBtree(pBt); if( rc!=SQLITE_OK ){ - *ppCur = 0; return rc; } } @@ -1572,14 +1576,6 @@ int sqlite3BtreeCursor( pCur->pNext->pPrev = pCur; } pCur->pPrev = 0; - pRing = pBt->pCursor; - while( pRing && pRing->pgnoRoot!=pCur->pgnoRoot ){ pRing = pRing->pNext; } - if( pRing ){ - pCur->pShared = pRing->pShared; - pRing->pShared = pCur; - }else{ - pCur->pShared = pCur; - } pBt->pCursor = pCur; pCur->isValid = 0; pCur->status = SQLITE_OK; @@ -1587,7 +1583,6 @@ int sqlite3BtreeCursor( return SQLITE_OK; create_cursor_exception: - *ppCur = 0; if( pCur ){ releasePage(pCur->pPage); sqliteFree(pCur); @@ -1625,11 +1620,6 @@ int sqlite3BtreeCloseCursor(BtCursor *pCur){ pCur->pNext->pPrev = pCur->pPrev; } releasePage(pCur->pPage); - if( pCur->pShared!=pCur ){ - BtCursor *pRing = pCur->pShared; - while( pRing->pShared!=pCur ){ pRing = pRing->pShared; } - pRing->pShared = pCur->pShared; - } unlockBtreeIfUnused(pBt); sqliteFree(pCur); return SQLITE_OK; @@ -3419,27 +3409,24 @@ static int balance(MemPage *pPage){ } /* -** This routine checks all cursors that point to the same table -** as pCur points to. If any of those cursors were opened with +** This routine checks all cursors that point to table pgnoRoot. +** If any of those cursors other than pExclude were opened with ** wrFlag==0 then this routine returns SQLITE_LOCKED. If all -** cursors point to the same table were opened with wrFlag==1 +** cursors that point to pgnoRoot were opened with wrFlag==1 ** then this routine returns SQLITE_OK. ** ** In addition to checking for read-locks (where a read-lock ** means a cursor opened with wrFlag==0) this routine also moves -** all cursors other than pCur so that they are pointing to the +** all cursors other than pExclude so that they are pointing to the ** first Cell on root page. This is necessary because an insert ** or delete might change the number of cells on a page or delete ** a page entirely and we do not want to leave any cursors ** pointing to non-existant pages or cells. */ -static int checkReadLocks(BtCursor *pCur){ +static int checkReadLocks(Btree *pBt, Pgno pgnoRoot, BtCursor *pExclude){ BtCursor *p; - assert( pCur->wrFlag ); - for(p=pCur->pShared; p!=pCur; p=p->pShared){ - assert( p ); - assert( p->pgnoRoot==pCur->pgnoRoot ); - assert( p->pPage->pgno==sqlite3pager_pagenumber(p->pPage->aData) ); + for(p=pBt->pCursor; p; p=p->pNext){ + if( p->pgnoRoot!=pgnoRoot || p==pExclude ) continue; if( p->wrFlag==0 ) return SQLITE_LOCKED; if( p->pPage->pgno!=p->pgnoRoot ){ moveToRoot(p); @@ -3481,7 +3468,7 @@ int sqlite3BtreeInsert( if( !pCur->wrFlag ){ return SQLITE_PERM; /* Cursor not open for writing */ } - if( checkReadLocks(pCur) ){ + if( checkReadLocks(pBt, pCur->pgnoRoot, pCur) ){ return SQLITE_LOCKED; /* The table pCur points to has a read lock */ } rc = sqlite3BtreeMoveto(pCur, pKey, nKey, &loc); @@ -3551,7 +3538,7 @@ int sqlite3BtreeDelete(BtCursor *pCur){ if( !pCur->wrFlag ){ return SQLITE_PERM; /* Did not open this cursor for writing */ } - if( checkReadLocks(pCur) ){ + if( checkReadLocks(pBt, pCur->pgnoRoot, pCur) ){ return SQLITE_LOCKED; /* The table pCur points to has a read lock */ } rc = sqlite3pager_write(pPage->aData); diff --git a/test/delete2.test b/test/delete2.test new file mode 100644 index 0000000000..597ae0f238 --- /dev/null +++ b/test/delete2.test @@ -0,0 +1,109 @@ +# 2003 September 6 +# +# 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. The +# focus of this script is a test to replicate the bug reported by +# ticket #842. +# +# Ticket #842 was a database corruption problem caused by a DELETE that +# removed an index entry by not the main table entry. To recreate the +# problem do this: +# +# (1) Create a table with an index. Insert some data into that table. +# (2) Start a query on the table but do not complete the query. +# (3) Try to delete a single entry from the table. +# +# Step 3 will fail because there is still a read cursor on the table. +# But the database is corrupted by the DELETE. It turns out that the +# index entry was deleted first, before the table entry. And the index +# delete worked. Thus an entry was deleted from the index but not from +# the table. +# +# The solution to the problem was to detect that the table is locked +# before the index entry is deleted. +# +# $Id: delete2.test,v 1.1 2004/08/08 19:43:30 drh Exp $ +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl + +# Create a table that has an index. +# +do_test delete2-1.1 { + db close + set DB [sqlite3 db test.db] + execsql { + CREATE TABLE q(s string, id string, constraint pk_q primary key(id)); + BEGIN; + INSERT INTO q(s,id) VALUES('hello','id.1'); + INSERT INTO q(s,id) VALUES('goodbye','id.2'); + INSERT INTO q(s,id) VALUES('again','id.3'); + END; + SELECT * FROM q; + } +} {hello id.1 goodbye id.2 again id.3} +do_test delete2-1.2 { + execsql { + SELECT * FROM q WHERE id='id.1'; + } +} {hello id.1} +do_test delete2-1.3 { + execsql { + PRAGMA integrity_check + } +} ok + +# Start a query on the table. The query should not use the index. +# Do not complete the query, thus leaving the table locked. +# +do_test delete2-1.4 { + set STMT [sqlite3_prepare $DB {SELECT * FROM q} -1 TAIL] + sqlite3_step $STMT +} SQLITE_ROW +do_test delete2-1.5 { + execsql {PRAGMA integrity_check} +} {ok} + +# Try to delete a row from the table. The delete should fail. +# +do_test delete2-1.6 { + catchsql { + DELETE FROM q WHERE rowid=1 + } +} {1 {database table is locked}} +do_test delete2-1.7 { + execsql {PRAGMA integrity_check} +} {ok} +do_test delete2-1.8 { + execsql { + SELECT * FROM q; + } +} {hello id.1 goodbye id.2 again id.3} + +# Finalize the query, thus clearing the lock on the table. Then +# retry the delete. The delete should work this time. +# +do_test delete2-1.9 { + sqlite3_finalize $STMT + catchsql { + DELETE FROM q WHERE rowid=1 + } +} {0 {}} +do_test delete2-1.10 { + execsql {PRAGMA integrity_check} +} {ok} +do_test delete2-1.11 { + execsql { + SELECT * FROM q; + } +} {goodbye id.2 again id.3} + +finish_test