Attempting to open a locked table for writing should fail immediately.

Ticket #842. (CVS 1880)

FossilOrigin-Name: fc879a9b1d05ddb8f8c552c1d334597e41b29b27

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)
+C Attempting\sto\sopen\sa\slocked\stable\sfor\swriting\sshould\sfail\simmediately.\r\nTicket\s#842.\s(CVS\s1880)
-D 2004-08-07T23:54:48
+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
+P d33771a303d9c20dd477b1a973024ff763203211
-R 537860faa651d2512143eecc05e62317
+R d49220a972e86ed5f2222c14670b9ccc
 U drh
-Z a6e6f8dc54fe5cdf4e420d0a0243394b
+Z 6bdb053e7f2204021eb022af1f2d8d7a

manifest.uuid

@@ -1 +1 @@
-d33771a303d9c20dd477b1a973024ff763203211
+fc879a9b1d05ddb8f8c552c1d334597e41b29b27

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;
-    *ppCur = 0;
+  if( wrFlag ){
-    return SQLITE_READONLY;
+    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
+** This routine checks all cursors that point to table pgnoRoot.
-** as pCur points to.  If any of those cursors were opened with
+** 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=pBt->pCursor; p; p=p->pNext){
-  for(p=pCur->pShared; p!=pCur; p=p->pShared){
+    if( p->pgnoRoot!=pgnoRoot || p==pExclude ) continue;
-    assert( p );
-    assert( p->pgnoRoot==pCur->pgnoRoot );
-    assert( p->pPage->pgno==sqlite3pager_pagenumber(p->pPage->aData) );
     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);

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