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
2009-07-02 17:21:57 +00:00 · 2009-07-02 17:21:57 +00:00 · 94b30733f7
commit 94b30733f7
parent 602b466e99
5 changed files with 127 additions and 48 deletions
--- a/15
+++ b/15
@ -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
--- a/manifest.uuid
+++ b/manifest.uuid
@ -1 +1 @@
-e3c055f167f895ae45858de9d9d8a264df2f36b6
+611e704fdf90a3d3932ca1cbab4be7e282bf1ddf
--- 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;
 }
--- 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.
--- a/test/sharedlock.test
+++ 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
+