Be more aggressive about using the busy handler. Ticket #1159. (CVS 2385)

FossilOrigin-Name: 644c6398e52481e5dda87671e1c196b26b1e4990

manifest

@@ -1,5 +1,5 @@
-C Fix\stypo\sin\sdocumentation.\s(CVS\s2384)
-D 2005-03-12T18:03:59
+C Be\smore\saggressive\sabout\susing\sthe\sbusy\shandler.\s\sTicket\s#1159.\s(CVS\s2385)
+D 2005-03-14T02:01:50
 F Makefile.in 5c00d0037104de2a50ac7647a5f12769795957a3
 F Makefile.linux-gcc 06be33b2a9ad4f005a5f42b22c4a19dab3cbb5c7
 F README a01693e454a00cc117967e3f9fdab2d4d52e9bc1
@@ -30,7 +30,7 @@ F sqlite3.pc.in 985b9bf34192a549d7d370e0f0b6b34a4f61369a
 F src/alter.c 6dab3d91aa4bf5c24e874145a2a547070c8c1883
 F src/attach.c f78f76bc6a8e5e487ca53636e21ccba2484a9a61
 F src/auth.c 18c5a0befe20f3a58a41e3ddd78f372faeeefe1f
-F src/btree.c fab5b169d25fef5a274b41edeff3f83eaa70fae2
+F src/btree.c 1d9b2179ccac13970c883da6ae3758cc72978bb0
 F src/btree.h 2e2cc923224649337d7217df0dd32b06673ca180
 F src/build.c a8792b2f866c1ccc32f4977f4ff61d787d60ddfb
 F src/date.c f3d1f5cd1503dabf426a198f3ebef5afbc122a7f
@@ -52,7 +52,7 @@ F src/os_unix.c d4823c6b3dd86e8cbb6a8f9d2fd6c4b3e722f8ee
 F src/os_unix.h 40b2fd1d02cfa45d6c3dea25316fd019cf9fcb0c
 F src/os_win.c bddeae1c3299be0fbe47077dd4e98b786a067f71
 F src/os_win.h 41a946bea10f61c158ce8645e7646b29d44f122b
-F src/pager.c a789b0ffe7a42fd99f54f8c938fd9a866669e79d
+F src/pager.c 2e795b3d85303b03daa1e18727a6aaa215a50980
 F src/pager.h 70d496f372163abb6340f474288c4bb9ea962cf7
 F src/parse.y 0b6135268a7a29db35335d5b71b5a8791e02f91e
 F src/pragma.c 4b20dbc0f4b97f412dc511853d3d0c2e0d4adedc
@@ -147,7 +147,7 @@ F test/join4.test cc6cafe85e11aacacd0abcd247a46bed251308f8
 F test/lastinsert.test b6a1db3e1ce2d3f0d6afe99d445084f543b6feaa
 F test/laststmtchanges.test 07cbdabc52407c29e40abc25050f2434f044a6b1
 F test/limit.test 270b076f31c5c32f7187de5727e74da4de43e477
-F test/lock.test a19aab9a963273fe61c1058e3d1b648d6a0a2425
+F test/lock.test 0b95ae28471f5123d24008d1c0fead911bf3c4be
 F test/lock2.test 59c3dd7d9b24d1bf7ec91b2d1541c37e97939d5f
 F test/lock3.test 615111293cf32aa2ed16d01c6611737651c96fb9
 F test/main.test febb69416071134dc38b9b1971c0c2e5b0ca3ff8
@@ -239,7 +239,7 @@ F www/audit.tcl 90e09d580f79c7efec0c7d6f447b7ec5c2dce5c0
 F www/autoinc.tcl b357f5ba954b046ee35392ce0f884a2fcfcdea06
 F www/c_interface.tcl b51b08591554c16a0c3ef718364a508ac25abc7e
 F www/capi3.tcl 7a7cc225fe02eb7ab861a6019b08baa0014409e1
-F www/capi3ref.tcl f395ff43a56ef087407b0e5ba6604f2f8c32cc14
+F www/capi3ref.tcl 59c7da9ef1f24dcb10b247cd3b7452cf76ab36f0
 F www/changes.tcl dbace6eb8ecf10f5e7047d77115feb1742e56610
 F www/common.tcl de758130d54d95d151ea0d17a2ae5b92e1bb01de
 F www/compile.tcl 65b26bdfc964b66c5f0af841718a52f9795ceb05
@@ -274,7 +274,7 @@ F www/tclsqlite.tcl e73f8f8e5f20e8277619433f7970060ab01088fc
 F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0
 F www/version3.tcl 092a01f5ef430d2c4acc0ae558d74c4bb89638a0
 F www/whentouse.tcl 3e522a06ad41992023c80ca29a048ae2331ca5bd
-P 33a0191638a4d6b33422f62487bfb9a0089d3cff
-R 2c190de0bffdef6a18303ed58cf43144
+P 78012246fc1c1fe844d192cfff69a736e388ce7a
+R 12c7fb7c202c4d99608b53151d0373a0
 U drh
-Z d98e1a0c532519fa0eb91c504a344f0c
+Z 9df5123a27f166217f6a13afcd69b178

manifest.uuid

@@ -1 +1 @@
-78012246fc1c1fe844d192cfff69a736e388ce7a
+644c6398e52481e5dda87671e1c196b26b1e4990

src/btree.c

@@ -9,7 +9,7 @@
 **    May you share freely, never taking more than you give.
 **
 *************************************************************************
-** $Id: btree.c,v 1.251 2005/03/10 17:06:34 drh Exp $
+** $Id: btree.c,v 1.252 2005/03/14 02:01:50 drh Exp $
 **
 ** This file implements a external (disk-based) database using BTrees.
 ** For a detailed discussion of BTrees, refer to
@@ -314,6 +314,7 @@ struct Btree {
   int minLocal;         /* Minimum local payload in non-LEAFDATA tables */
   int maxLeaf;          /* Maximum local payload in a LEAFDATA table */
   int minLeaf;          /* Minimum local payload in a LEAFDATA table */
+  BusyHandler *pBusyHandler;   /* Callback for when there is lock contention */
 };
 typedef Btree Bt;
 
@@ -1291,6 +1292,7 @@ int sqlite3BtreeClose(Btree *pBt){
 ** Change the busy handler callback function.
 */
 int sqlite3BtreeSetBusyHandler(Btree *pBt, BusyHandler *pHandler){
+  pBt->pBusyHandler = pHandler;
   sqlite3pager_set_busyhandler(pBt->pPager, pHandler);
   return SQLITE_OK;
 }
@@ -1479,6 +1481,20 @@ page1_init_failed:
   return rc;
 }
 
+/*
+** This routine works like lockBtree() except that it also invokes the
+** busy callback if there is lock contention.
+*/
+static int lockBtreeWithRetry(Btree *pBt){
+  int rc = SQLITE_OK;
+  if( pBt->inTrans==TRANS_NONE ){
+    rc = sqlite3BtreeBeginTrans(pBt, 0);
+    pBt->inTrans = TRANS_NONE;
+  }
+  return rc;
+}
+       
+
 /*
 ** If there are no outstanding cursors and we are not in the middle
 ** of a transaction but there is a read lock on the database, then
@@ -1543,7 +1559,7 @@ static int newDatabase(Btree *pBt){
 ** transaction.  If the second argument is 2 or more and exclusive
 ** transaction is started, meaning that no other process is allowed
 ** to access the database.  A preexisting transaction may not be
-** upgrade to exclusive by calling this routine a second time - the
+** upgraded to exclusive by calling this routine a second time - the
 ** exclusivity flag only works for a new transaction.
 **
 ** A write-transaction must be started before attempting any 
@@ -1558,26 +1574,39 @@ static int newDatabase(Btree *pBt){
 **      sqlite3BtreeDelete()
 **      sqlite3BtreeUpdateMeta()
 **
-** If wrflag is true, then nMaster specifies the maximum length of
-** a master journal file name supplied later via sqlite3BtreeSync().
-** This is so that appropriate space can be allocated in the journal file
-** when it is created..
+** If an initial attempt to acquire the lock fails because of lock contention
+** and the database was previously unlocked, then invoke the busy handler
+** if there is one.  But if there was previously a read-lock, do not
+** invoke the busy handler - just return SQLITE_BUSY.  SQLITE_BUSY is 
+** returned when there is already a read-lock in order to avoid a deadlock.
+**
+** Suppose there are two processes A and B.  A has a read lock and B has
+** a reserved lock.  B tries to promote to exclusive but is blocked because
+** of A's read lock.  A tries to promote to reserved but is blocked by B.
+** One or the other of the two processes must give way or there can be
+** no progress.  By returning SQLITE_BUSY and not invoking the busy callback
+** when A already has a read lock, we encourage A to give up and let B
+** proceed.
 */
 int sqlite3BtreeBeginTrans(Btree *pBt, int wrflag){
   int rc = SQLITE_OK;
+  int busy = 0;
+  BusyHandler *pH;
 
   /* If the btree is already in a write-transaction, or it
   ** is already in a read-transaction and a read-transaction
   ** is requested, this is a no-op.
   */
-  if( pBt->inTrans==TRANS_WRITE || 
-      (pBt->inTrans==TRANS_READ && !wrflag) ){
+  if( pBt->inTrans==TRANS_WRITE || (pBt->inTrans==TRANS_READ && !wrflag) ){
     return SQLITE_OK;
   }
+
+  /* Write transactions are not possible on a read-only database */
   if( pBt->readOnly && wrflag ){
     return SQLITE_READONLY;
   }
 
+  do {
     if( pBt->pPage1==0 ){
       rc = lockBtree(pBt);
     }
@@ -1595,6 +1624,10 @@ int sqlite3BtreeBeginTrans(Btree *pBt, int wrflag){
     }else{
       unlockBtreeIfUnused(pBt);
     }
+  }while( rc==SQLITE_BUSY && pBt->inTrans==TRANS_NONE &&
+      (pH = pBt->pBusyHandler)!=0 && 
+      pH->xFunc && pH->xFunc(pH->pArg, busy++)
+  );
   return rc;
 }
 
@@ -2116,7 +2149,7 @@ int sqlite3BtreeCursor(
     }
   }
   if( pBt->pPage1==0 ){
-    rc = lockBtree(pBt);
+    rc = lockBtreeWithRetry(pBt);
     if( rc!=SQLITE_OK ){
       return rc;
     }
@@ -5531,7 +5564,7 @@ char *sqlite3BtreeIntegrityCheck(Btree *pBt, int *aRoot, int nRoot){
   IntegrityCk sCheck;
 
   nRef = *sqlite3pager_stats(pBt->pPager);
-  if( lockBtree(pBt)!=SQLITE_OK ){
+  if( lockBtreeWithRetry(pBt)!=SQLITE_OK ){
     return sqliteStrDup("Unable to acquire a read lock on the database");
   }
   sCheck.pBt = pBt;

src/pager.c

@@ -18,7 +18,7 @@
 ** file simultaneously, or one process from reading the database while
 ** another is writing.
 **
-** @(#) $Id: pager.c,v 1.192 2005/03/10 14:11:13 drh Exp $
+** @(#) $Id: pager.c,v 1.193 2005/03/14 02:01:50 drh Exp $
 */
 #include "sqliteInt.h"
 #include "os.h"
@@ -1824,12 +1824,12 @@ static int pager_wait_on_lock(Pager *pPager, int locktype){
     rc = SQLITE_OK;
   }else{
     int busy = 1;
+    BusyHandler *pH;
     do {
       rc = sqlite3OsLock(&pPager->fd, locktype);
     }while( rc==SQLITE_BUSY && 
-        pPager->pBusyHandler && 
-        pPager->pBusyHandler->xFunc && 
-        pPager->pBusyHandler->xFunc(pPager->pBusyHandler->pArg, busy++)
+        (pH = pPager->pBusyHandler)!=0 && 
+        pH->xFunc && pH->xFunc(pH->pArg, busy++)
     );
     if( rc==SQLITE_OK ){
       pPager->state = locktype;
@@ -2632,12 +2632,8 @@ int sqlite3pager_begin(void *pData, int exFlag){
     if( MEMDB ){
       pPager->state = PAGER_EXCLUSIVE;
       pPager->origDbSize = pPager->dbSize;
-    }else{
-      if( SQLITE_BUSY_RESERVED_LOCK || exFlag ){
-        rc = pager_wait_on_lock(pPager, RESERVED_LOCK);
     }else{
       rc = sqlite3OsLock(&pPager->fd, RESERVED_LOCK);
-      }
       if( rc==SQLITE_OK ){
         pPager->state = PAGER_RESERVED;
         if( exFlag ){

test/lock.test

@@ -11,7 +11,7 @@
 # This file implements regression tests for SQLite library.  The
 # focus of this script is database locks.
 #
-# $Id: lock.test,v 1.30 2005/01/12 12:44:04 danielk1977 Exp $
+# $Id: lock.test,v 1.31 2005/03/14 02:01:50 drh Exp $
 
 
 set testdir [file dirname $argv0]
@@ -169,30 +169,56 @@ do_test lock-2.2 {
 } {0 {9 8}}
 
 # If the other thread (the one that does not hold the transaction with
-# a RESERVED lock) tries to get a RESERVED lock, we do not get a busy callback.
+# a RESERVED lock) tries to get a RESERVED lock, we do get a busy callback
+# as long as we were not orginally holding a READ lock.
 #
-do_test lock-2.3 {
+do_test lock-2.3.1 {
   proc callback {count} {
     set ::callback_value $count
     break
   }
   set ::callback_value {}
   db2 busy callback
+  # db2 does not hold a lock so we should get a busy callback here
+  set r [catch {execsql {UPDATE t1 SET a=b, b=a} db2} msg]
+  lappend r $msg
+  lappend r $::callback_value
+} {1 {database is locked} 0}
+do_test lock-2.3.2 {
+  set ::callback_value {}
+  execsql {BEGIN; SELECT rowid FROM sqlite_master LIMIT 1} db2
+  # This time db2 does hold a read lock.  No busy callback this time.
   set r [catch {execsql {UPDATE t1 SET a=b, b=a} db2} msg]
   lappend r $msg
   lappend r $::callback_value
 } {1 {database is locked} {}}
-do_test lock-2.4 {
+catch {execsql {ROLLBACK} db2}
+do_test lock-2.4.1 {
   proc callback {count} {
     lappend ::callback_value $count
     if {$count>4} break
   }
   set ::callback_value {}
   db2 busy callback
+  # We get a busy callback because db2 is not holding a lock
+  set r [catch {execsql {UPDATE t1 SET a=b, b=a} db2} msg]
+  lappend r $msg
+  lappend r $::callback_value
+} {1 {database is locked} {0 1 2 3 4 5}}
+do_test lock-2.4.2 {
+  proc callback {count} {
+    lappend ::callback_value $count
+    if {$count>4} break
+  }
+  set ::callback_value {}
+  db2 busy callback
+  execsql {BEGIN; SELECT rowid FROM sqlite_master LIMIT 1} db2
+  # No busy callback this time because we are holding a lock
   set r [catch {execsql {UPDATE t1 SET a=b, b=a} db2} msg]
   lappend r $msg
   lappend r $::callback_value
 } {1 {database is locked} {}}
+catch {execsql {ROLLBACK} db2}
 do_test lock-2.5 {
   proc callback {count} {
     lappend ::callback_value $count
@@ -255,7 +281,7 @@ do_test lock-4.3 {
   db2 busy callback
   set rc [catch {db2 eval {UPDATE t1 SET a=0}} msg]
   lappend rc $msg $::callback_value
-} {1 {database is locked} {}}
+} {1 {database is locked} {0 1 2 3 4 5}}
 execsql {ROLLBACK}
 
 # When one thread is writing, other threads cannot read.  Except if the

www/capi3ref.tcl

@@ -1,4 +1,4 @@
-set rcsid {$Id: capi3ref.tcl,v 1.19 2005/03/12 18:03:59 drh Exp $}
+set rcsid {$Id: capi3ref.tcl,v 1.20 2005/03/14 02:01:50 drh Exp $}
 source common.tcl
 header {C/C++ Interface For SQLite Version 3}
 puts {
@@ -165,7 +165,7 @@ api {} {
  If the callback returns non-zero, then another attempt is made to open the
  database for reading and the cycle repeats.
 
- That a busy handler is registered does not guarantee that
+ The presence of a busy handler does not guarantee that
  it will be invoked when there is lock contention.
  If SQLite determines that invoking the busy handler could result in
  a deadlock, it will return SQLITE_BUSY instead.