More work on windows locking. Fix some problems with unix locking. There

is still an assertion failure on windows locking in attach2.test. (CVS 1539)

FossilOrigin-Name: 0c2d169cf3c0f36972015c952a2b46cb9a333881

manifest

@@ -1,5 +1,5 @@
-C Tables\screated\swith\sthe\sCREATE\sTABLE\s<tbl>\sAS\sSELECT\s...\ssyntax\snow\sinherit\ncolumn\sdeclaration\stypes\sfrom\sthe\sSELECT\sstatement.\s(CVS\s1538)
+C More\swork\son\swindows\slocking.\s\sFix\ssome\sproblems\swith\sunix\slocking.\s\sThere\nis\sstill\san\sassertion\sfailure\son\swindows\slocking\sin\sattach2.test.\s(CVS\s1539)
-D 2004-06-07T10:00:31
+D 2004-06-07T16:27:46
 F Makefile.in ab7b0d5118e2da97bac66be8684a1034e3500f5a
 F Makefile.linux-gcc b86a99c493a5bfb402d1d9178dcdc4bd4b32f906
 F README f1de682fbbd94899d50aca13d387d1b3fd3be2dd
@@ -25,7 +25,7 @@ F sqlite.def fc4f5734786fe4743cfe2aa98eb2da4b089edb5f
 F sqlite.pc.in 30552343140c53304c2a658c080fbe810cd09ca2
 F src/attach.c e76e4590ec5dd389e5646b171881b5243a6ef391
 F src/auth.c 5c2f0bea4729c98c2be3b69d6b466fc51448fe79
-F src/btree.c 3f0c22ab8c0c000ee5de4ad875836c111c4191c6
+F src/btree.c edb38affc2e83f4299e49104cfe14e6570d8bd32
 F src/btree.h 589427ac13bb544d298cd99726e2572a6fe4bdaa
 F src/build.c e8043747608eba9ff293f4fcb52cbf958625d599
 F src/date.c 8e6fa3173386fb29fdef012ee08a853c1e9908b2
@@ -39,19 +39,19 @@ F src/insert.c 4268d9e3959cc845ea243fb4ec7507269404dad9
 F src/legacy.c ad23746f15f67e34577621b1875f639c94839e1f
 F src/main.c d34e173296473c9626f2560a0c86e694fc8e5a2b
 F src/md5.c 4302e84ae516c616bb079c4e6d038c0addb33481
-F src/os.h 4e480eb92737ebcdd1e1136bdbf5cd22223bd1b4
+F src/os.h a3a9c2df29acbff54aef742e85c302d23634019f
-F src/os_common.h 7b0f4ae0d9f66888f90ab28f126b42bfefe0bbd4
+F src/os_common.h 12074232439f904b3805beeff1245bd1b5532994
 F src/os_mac.c b823874690615ace0dd520d3ad1fe8bfd864b7e0
 F src/os_mac.h 51d2445f47e182ed32d3bd6937f81070c6fd9bd4
-F src/os_unix.c a4feb70b23fa5272f53cd2c74588484b54294800
+F src/os_unix.c f78604d00c0769e4ffa481dba110cdd098cc276b
 F src/os_unix.h 7999f2246c6347707e98f7078871ea8ca605df3f
-F src/os_win.c 3c21eddf78e98d7be7dde1bf156d97e1d8ddd63d
+F src/os_win.c 5c46f81f76f2be0c0817773a865c075c56496018
 F src/os_win.h 004eec47b1780fcaf07420ddc2072294b698d48c
-F src/pager.c 944f6b071279887574081281f27bb2af88b42905
+F src/pager.c 3fddd1e5b3e449b19e4f762ab1f1d10786d56d28
 F src/pager.h 0c7b5ac45c69e690c45d160d03bdc8fbd2d4657b
 F src/parse.y 27c1ce09f9d309be91f9e537df2fb00892990af4
 F src/pragma.c 54b4d67fa81fd38b911aa3325348dcae9ceac5a4
-F src/printf.c 77ee9ec6dbf1b7512b17d63ccf8322ea9466278b
+F src/printf.c 63b15f1ea9fe3daa066bb7430fd20d4a2d717dc8
 F src/random.c eff68e3f257e05e81eae6c4d50a51eb88beb4ff3
 F src/select.c 0ac0adeb2ae15255ac4399d9ee1b0d25a266a676
 F src/shell.c 79af86d39b2149c7f16219fcbe636e7c2da9df8e
@@ -59,7 +59,7 @@ F src/sqlite.h.in 4705697dd7213f322d59ffc69b48b8ac32b23373
 F src/sqliteInt.h 845d2a3ffdb9a9050a1b55044d4856227b649b84
 F src/table.c af14284fa36c8d41f6829e3f2819dce07d3e2de2
 F src/tclsqlite.c 3db6b868bd844bfb71720c8e573f4c9b0d536bd5
-F src/test1.c 4a3cc1b628a29f24c0a43227a035d0f2a96eb634
+F src/test1.c b2498201826d83c8240a205954ecf8fc1b1d3554
 F src/test2.c ae18537d8a85e5028c955837797f9da461b908b8
 F src/test3.c beafd0ccf7b9ae784744be1b1e66ffe8f64c25da
 F src/test4.c caf675e443460ec76b04d78e1688986c17c82cec
@@ -74,12 +74,12 @@ F src/vdbe.c 392c6b02c525ea12dff403ba4ceb42b0afcb42f5
 F src/vdbe.h 46f74444a213129bc4b5ce40124dd8ed613b0cde
 F src/vdbeInt.h ab592f23ed5a1913f9a506bd7b76c5e39377942a
 F src/vdbeapi.c 4ac95766b0515538037a7aec172ed26142f97cf9
-F src/vdbeaux.c c47fd5433a965b5f06de2498a56401861ce5ecbe
+F src/vdbeaux.c cd1be846336f039442503991fa2aba70f1708554
 F src/vdbemem.c 5d029d83bc60eaf9c45837fcbc0b03348ec95d7a
 F src/where.c 444a7c3a8b1eb7bba072e489af628555d21d92a4
 F test/all.test 569a92a8ee88f5300c057cc4a8f50fbbc69a3242
 F test/attach.test 1635022d7e1d95dc92fe381cc62f9bf25cb29d73
-F test/attach2.test e98aab312722d05fc1837bf103baeebc582c64f8
+F test/attach2.test 2185dce04ef9ceb7b2d3df7d17fb2c3817028dea
 F test/attach3.test 8259ab833b5dcdf4acd75d9653f42f703ce2e013
 F test/auth.test 95809b8f6a9bec18b94d28cafd03fe27d2f8a9e9
 F test/bigfile.test ea904b853ce2d703b16c5ce90e2b54951bc1ae81
@@ -215,7 +215,7 @@ F www/support.tcl 1801397edd271cc39a2aadd54e701184b5181248
 F www/tclsqlite.tcl 19191cf2a1010eaeff74c51d83fd5f5a4d899075
 F www/vdbe.tcl 59288db1ac5c0616296b26dce071c36cb611dfe9
 F www/whentouse.tcl a8335bce47cc2fddb07f19052cb0cb4d9129a8e4
-P 983221b038813c5a7892564896e39597c223c4c3
+P 31c1668dbc2b84438a5b78b0270f58b37b03791d
-R 9252a60acb4c967bd956d4d623f3bcb8
+R 334ce43bb39f3c00215ba0df14f1b502
-U danielk1977
+U drh
-Z b067b126b336d1aa6d04e5d165e845dc
+Z f7c5a4dea192989af98a28345518141d

manifest.uuid

@@ -1 +1 @@
-31c1668dbc2b84438a5b78b0270f58b37b03791d
+0c2d169cf3c0f36972015c952a2b46cb9a333881

src/btree.c

@@ -9,7 +9,7 @@
 **    May you share freely, never taking more than you give.
 **
 *************************************************************************
-** $Id: btree.c,v 1.160 2004/06/07 01:52:14 drh Exp $
+** $Id: btree.c,v 1.161 2004/06/07 16:27:46 drh Exp $
 **
 ** This file implements a external (disk-based) database using BTrees.
 ** For a detailed discussion of BTrees, refer to
@@ -3713,12 +3713,6 @@ int sqlite3BtreeFlags(BtCursor *pCur){
   return pPage ? pPage->aData[pPage->hdrOffset] : 0;
 }
 
-/******************************************************************************
-** The complete implementation of the BTree subsystem is above this line.
-** All the code the follows is for testing and troubleshooting the BTree
-** subsystem.  None of the code that follows is used during normal operation.
-******************************************************************************/
-
 /*
 ** Print a disassembly of the given page on standard output.  This routine
 ** is used for debugging and testing only.

src/os.h

@@ -90,6 +90,57 @@
 #define PENDING_LOCK    3
 #define EXCLUSIVE_LOCK  4
 
+/*
+** Windows file locking notes:
+**
+** We cannot use LockFileEx() or UnlockFileEx() on Win95/98/ME because
+** those functions are not available.  So we use only LockFile() and
+** UnlockFile().
+**
+** LockFile() prevents not just writing but also reading by other processes.
+** (This is a design error on the part of Windows, but there is nothing
+** we can do about that.)  So the region used for locking is at the
+** end of the file where it is unlikely to ever interfere with an
+** actual read attempt.
+**
+** A SHARED_LOCK is obtained by locking a single randomly-chosen 
+** byte out of a specific range of bytes. The lock byte is obtained at 
+** random so two separate readers can probably access the file at the 
+** same time, unless they are unlucky and choose the same lock byte.
+** An EXCLUSIVE_LOCK is obtained by locking all bytes in the range.
+** There can only be one writer.  A RESERVED_LOCK is obtained by locking
+** a single byte of the file that is designated as the reserved lock byte.
+** A PENDING_LOCK is obtained by locking a designated byte different from
+** the RESERVED_LOCK byte.
+**
+** On WinNT/2K/XP systems, LockFileEx() and UnlockFileEx() are available,
+** which means we can use reader/writer locks.  When reader/writer locks
+** are used, the lock is placed on the same range of bytes that is used
+** for probabilistic locking in Win95/98/ME.  Hence, the locking scheme
+** will support two or more Win95 readers or two or more WinNT readers.
+** But a single Win95 reader will lock out all WinNT readers and a single
+** WinNT reader will lock out all other Win95 readers.
+**
+** The following #defines specify the range of bytes used for locking.
+** SHARED_SIZE is the number of bytes available in the pool from which
+** a random byte is selected for a shared lock.  The pool of bytes for
+** shared locks begins at SHARED_FIRST. 
+**
+** These #defines are available in os.h so that Unix can use the same
+** byte ranges for locking.  This leaves open the possiblity of having
+** clients on win95, winNT, and unix all talking to the same shared file
+** and all locking correctly.
+**
+** Locking in windows is manditory.  For this reason, we cannot store
+** actual data in the bytes used for locking.  The pager never allocates
+** the pages involved in locking therefore.
+*/
+#define SHARED_SIZE       10238
+#define SHARED_FIRST      (0x3fffffff - (SHARED_SIZE - 1))
+#define RESERVED_BYTE     (SHARED_FIRST - 1)
+#define PENDING_BYTE      (RESERVED_BYTE - 1)
+
+
 int sqlite3OsDelete(const char*);
 int sqlite3OsFileExists(const char*);
 int sqliteOsFileRename(const char*, const char*);

src/os_common.h

@@ -23,7 +23,8 @@
 ** Macros for performance tracing.  Normally turned off.  Only works
 ** on i486 hardware.
 */
-#if 0
+int sqlite3_os_trace = 0;
+#if 1
 static int last_page = 0;
 __inline__ unsigned long long int hwtime(void){
   unsigned long long int x;
@@ -37,11 +38,11 @@ static unsigned int elapse;
 #define TIMER_START       g_start=hwtime()
 #define TIMER_END         elapse=hwtime()-g_start
 #define SEEK(X)           last_page=(X)
-#define TRACE1(X)         sqlite3DebugPrintf(X)
+#define TRACE1(X)         if( sqlite3_os_trace ) sqlite3DebugPrintf(X)
-#define TRACE2(X,Y)       sqlite3DebugPrintf(X,Y)
+#define TRACE2(X,Y)       if( sqlite3_os_trace ) sqlite3DebugPrintf(X,Y)
-#define TRACE3(X,Y,Z)     sqlite3DebugPrintf(X,Y,Z)
+#define TRACE3(X,Y,Z)     if( sqlite3_os_trace ) sqlite3DebugPrintf(X,Y,Z)
-#define TRACE4(X,Y,Z,A)   sqlite3DebugPrintf(X,Y,Z,A)
+#define TRACE4(X,Y,Z,A)   if( sqlite3_os_trace ) sqlite3DebugPrintf(X,Y,Z,A)
-#define TRACE5(X,Y,Z,A,B) sqlite3DebugPrintf(X,Y,Z,A,B)
+#define TRACE5(X,Y,Z,A,B) if( sqlite3_os_trace ) sqlite3DebugPrintf(X,Y,Z,A,B)
 #else
 #define TIMER_START
 #define TIMER_END

src/os_unix.c

@@ -174,7 +174,7 @@ struct lockKey {
 */
 struct lockInfo {
   struct lockKey key;  /* The lookup key */
-  int cnt;             /* Number of locks held */
+  int cnt;             /* Number of SHARED locks held */
   int locktype;        /* One of SHARED_LOCK, RESERVED_LOCK etc. */
   int nRef;            /* Number of pointers to this structure */
 };
@@ -644,27 +644,27 @@ int sqlite3OsFileSize(OsFile *id, off_t *pSize){
 /*
 ** This routine checks if there is a RESERVED lock held on the specified
 ** file by this or any other process. If such a lock is held, return
-** non-zero, otherwise zero.
+** non-zero.  If the file is unlocked or holds only SHARED locks, then
+** return zero.
 */
 int sqlite3OsCheckWriteLock(OsFile *id){
   int r = 0;
 
-  sqlite3OsEnterMutex();
+  sqlite3OsEnterMutex(); /* Needed because id->pLock is shared across threads */
 
   /* Check if a thread in this process holds such a lock */
   if( id->pLock->locktype>SHARED_LOCK ){
     r = 1;
   }
 
-  /* Otherwise see if some other process holds it. Just check the whole
+  /* Otherwise see if some other process holds it.
-  ** file for write-locks, rather than any specific bytes.
   */
   if( !r ){
     struct flock lock;
     lock.l_whence = SEEK_SET;
-    lock.l_start = 0;
+    lock.l_start = RESERVED_BYTE;
-    lock.l_len = 0;
+    lock.l_len = 1;
-    lock.l_type = F_RDLCK;
+    lock.l_type = F_WRLCK;
     fcntl(id->fd, F_GETLK, &lock);
     if( lock.l_type!=F_UNLCK ){
       r = 1;
@@ -672,6 +672,7 @@ int sqlite3OsCheckWriteLock(OsFile *id){
   }
   
   sqlite3OsLeaveMutex();
+  TRACE3("TEST WR-LOCK %d %d\n", id->fd, r);
 
   return r;
 }
@@ -680,10 +681,25 @@ int sqlite3OsCheckWriteLock(OsFile *id){
 ** Lock the file with the lock specified by parameter locktype - one
 ** of the following:
 **
-** SHARED_LOCK
+**     (1) SHARED_LOCK
-** RESERVED_LOCK
+**     (2) RESERVED_LOCK
-** PENDING_LOCK
+**     (3) PENDING_LOCK
-** EXCLUSIVE_LOCK
+**     (4) EXCLUSIVE_LOCK
+**
+** Locks are are hierarchical.  Getting a lock N implies getting all locks
+** N-1, N-2, N-3, ....  So, for example, getting a PENDING lock
+** implies a SHARED and a RESERVED lock.  This routine adds locks one
+** at a time until the desired lock is acheived.  A locking failure might
+** occur at any point.  When a failure occurs intermediate locks are
+** retained.  For example, if a SHARED lock is held and this routine
+** is called with EXCLUSIVE, it might obtain a RESERVED and PENDING lock
+** but fail to get the EXCLUSIVE lock.  In that case, the file would be
+** left in the PENDING lock state - it does not revert to SHARED.
+**
+** This routine will only increase a lock.  The sqlite3OsUnlock() routine
+** erases all locks at once and returns us immediately to locking level 0.
+** It is not possible to lower the locking level one step at a time.  You
+** must go straight to locking level 0.
 */
 int sqlite3OsLock(OsFile *id, int locktype){
   int rc = SQLITE_OK;
@@ -691,12 +707,8 @@ int sqlite3OsLock(OsFile *id, int locktype){
   struct flock lock;
   int s;
 
-  /* It is an error to request any kind of lock before a shared lock */
+  TRACE5("LOCK %d %d was %d(%d)\n",
-  if( locktype>SHARED_LOCK && id->locktype==0 ){
+          id->fd, locktype, id->locktype, pLock->locktype);
-    rc = sqlite3OsLock(id, SHARED_LOCK);
-    if( rc!=SQLITE_OK ) return rc;
-  }
-  assert( locktype==SHARED_LOCK || id->locktype!=0 );
 
   /* If there is already a lock of this type or more restrictive on the
   ** OsFile, do nothing. Don't use the end_lock: exit path, as
@@ -706,14 +718,27 @@ int sqlite3OsLock(OsFile *id, int locktype){
     return SQLITE_OK;
   }
 
-  sqlite3OsEnterMutex();
+  /* Make sure locking is sequential.  In other words, make sure we have
+  ** SHARED before trying for RESERVED, and that we have RESERVED before
+  ** trying for PENDING, and that we have PENDING before trying for
+  ** EXCLUSIVE.
+  */
+  while( locktype>id->locktype+1 ){
+    rc = sqlite3OsLock(id, id->locktype+1);
+    if( rc!=SQLITE_OK ){
+      return rc;
+    }
+  }
+  assert( locktype==id->locktype+1 );
+
+  sqlite3OsEnterMutex();  /* Needed because pLock is shared across threads */
 
   /* If some thread using this PID has a lock via a different OsFile*
   ** handle that precludes the requested lock, return BUSY.
   */
   if( (id->locktype!=pLock->locktype && 
-      (pLock->locktype>RESERVED_LOCK || locktype!=SHARED_LOCK)) ||
+          (pLock->locktype>=PENDING_LOCK || locktype>SHARED_LOCK))
-      (locktype>RESERVED_LOCK && pLock->cnt>1)
+   || (locktype==EXCLUSIVE_LOCK && pLock->cnt>1)
   ){
     rc = SQLITE_BUSY;
     goto end_lock;
@@ -744,21 +769,25 @@ int sqlite3OsLock(OsFile *id, int locktype){
     assert( pLock->cnt==0 );
     assert( pLock->locktype==0 );
   
-    /* Grab a read-lock on byte 2. This ensures that no other process
+    /* Temporarily grab a PENDING lock.  This prevents new SHARED locks from
-    ** has a PENDING lock.
+    ** being formed if a PENDING lock is already held.
     */
     lock.l_type = F_RDLCK;
-    lock.l_start = 2;
+    lock.l_start = PENDING_BYTE;
     s = fcntl(id->fd, F_SETLK, &lock);
     if( s ){
       rc = (errno==EINVAL) ? SQLITE_NOLFS : SQLITE_BUSY;
       goto end_lock;
     }
 
-    /* Now get a read-lock on byte 0 and renege on the byte 2 lock. */
+    /* Now get the read-lock */
-    lock.l_start = 0;
+    lock.l_start = SHARED_FIRST;
+    lock.l_len = SHARED_SIZE;
     s = fcntl(id->fd, F_SETLK, &lock);
-    lock.l_start = 2;
+
+    /* Drop the temporary PENDING lock */
+    lock.l_start = PENDING_BYTE;
+    lock.l_len = 1L;
     lock.l_type = F_UNLCK;
     fcntl(id->fd, F_SETLK, &lock);
     if( s ){
@@ -777,13 +806,14 @@ int sqlite3OsLock(OsFile *id, int locktype){
     lock.l_type = F_WRLCK;
     switch( locktype ){
       case RESERVED_LOCK:
-        lock.l_start = 1;
+        lock.l_start = RESERVED_BYTE;
         break;
       case PENDING_LOCK:
-        lock.l_start = 2;
+        lock.l_start = PENDING_BYTE;
         break;
       case EXCLUSIVE_LOCK:
-        lock.l_start = 0;
+        lock.l_start = SHARED_FIRST;
+        lock.l_len = SHARED_SIZE;
         break;
       default:
         assert(0);
@@ -797,11 +827,11 @@ int sqlite3OsLock(OsFile *id, int locktype){
   if( rc==SQLITE_OK ){
     id->locktype = locktype;
     pLock->locktype = locktype;
-    assert( pLock->locktype==RESERVED_LOCK || pLock->cnt==1 );
   }
 
 end_lock:
   sqlite3OsLeaveMutex();
+  TRACE4("LOCK %d %d %s\n", id->fd, locktype, rc==SQLITE_OK ? "ok" : "failed");
   return rc;
 }
 

src/os_win.c

@@ -313,47 +313,6 @@ int sqlite3OsFileSize(OsFile *id, off_t *pSize){
   return SQLITE_OK;
 }
 
-/*
-** Windows file locking notes:
-**
-** We cannot use LockFileEx() or UnlockFileEx() on Win95/98/ME because
-** those functions are not available.  So we use only LockFile() and
-** UnlockFile().
-**
-** LockFile() prevents not just writing but also reading by other processes.
-** (This is a design error on the part of Windows, but there is nothing
-** we can do about that.)  So the region used for locking is at the
-** end of the file where it is unlikely to ever interfere with an
-** actual read attempt.
-**
-** A SHARED_LOCK is obtained by locking a single randomly-chosen 
-** byte out of a specific range of bytes. The lock byte is obtained at 
-** random so two separate readers can probably access the file at the 
-** same time, unless they are unlucky and choose the same lock byte.
-** An EXCLUSIVE_LOCK is obtained by locking all bytes in the range.
-** There can only be one writer.  A RESERVED_LOCK is obtained by locking
-** a single byte of the file that is designated as the reserved lock byte.
-** A PENDING_LOCK is obtained by locking a designated byte different from
-** the RESERVED_LOCK byte.
-**
-** On WinNT/2K/XP systems, LockFileEx() and UnlockFileEx() are available,
-** which means we can use reader/writer locks.  When reader/writer locks
-** are used, the lock is placed on the same range of bytes that is used
-** for probabilistic locking in Win95/98/ME.  Hence, the locking scheme
-** will support two or more Win95 readers or two or more WinNT readers.
-** But a single Win95 reader will lock out all WinNT readers and a single
-** WinNT reader will lock out all other Win95 readers.
-**
-** The following #defines specify the range of bytes used for locking.
-** SHARED_SIZE is the number of bytes available in the pool from which
-** a random byte is selected for a shared lock.  The pool of bytes for
-** shared locks begins at SHARED_FIRST.  
-*/
-#define SHARED_SIZE       10238
-#define SHARED_FIRST      (0x3fffffff - (SHARED_SIZE - 1))
-#define RESERVED_BYTE     (SHARED_FIRST - 1)
-#define PENDING_BYTE      (RESERVED_BYTE - 1)
-
 /*
 ** Return true (non-zero) if we are running under WinNT, Win2K or WinXP.
 ** Return false (zero) for Win95, Win98, or WinME.
@@ -531,13 +490,16 @@ int sqlite3OsCheckWriteLock(OsFile *id){
   int rc;
   if( id->locktype>=RESERVED_LOCK ){
     rc = 1;
+    TRACE3("TEST WR-LOCK %d %d (local)\n", id->h, rc);
   }else{
     rc = LockFile(id->h, RESERVED_BYTE, 0, 1, 0);
     if( rc ){
       UnlockFile(id->h, RESERVED_BYTE, 0, 1, 0);
     }
+    rc = !rc;
+    TRACE3("TEST WR-LOCK %d %d (remote)\n", id->h, rc);
   }
-  return 0;
+  return rc;
 }
 
 /*

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.112 2004/06/04 10:38:31 danielk1977 Exp $
+** @(#) $Id: pager.c,v 1.113 2004/06/07 16:27:46 drh Exp $
 */
 #include "os.h"         /* Must be first to enable large file support */
 #include "sqliteInt.h"
@@ -1554,20 +1554,16 @@ static int pager_write_pagelist(PgHdr *pList){
   ** database file. If there is already an EXCLUSIVE lock, the following
   ** calls to sqlite3OsLock() are no-ops.
   **
-  ** The upgrade from a RESERVED to PENDING lock cannot return SQLITE_BUSY,
+  ** The upgrade from a RESERVED to PENDING might return SQLITE_BUSY on
-  ** unless someone is not following the locking protocol. 
+  ** windows because the windows locking mechanism acquires a transient
+  ** PENDING lock during its attempts to get a SHARED lock.  So if another
+  ** process were trying to get a SHARED lock at the same time this process
+  ** is upgrading from RESERVED to PENDING, the two could collide.
   **
-  ** The upgrade from PENDING to EXCLUSIVE can return SQLITE_BUSY. It's
+  ** The upgrade from PENDING to EXCLUSIVE can return SQLITE_BUSY if there
-  ** not totally clear that the busy-callback should be invoked here
+  ** are still active readers that were created before the PENDING lock
-  ** though. (?)
+  ** was acquired.
   */
-  rc = sqlite3OsLock(&pPager->fd, PENDING_LOCK);
-  if( rc==SQLITE_BUSY ){
-    return SQLITE_PROTOCOL;
-  }
-  if( rc!=SQLITE_OK ){
-    return rc;
-  }
   do {
     rc = sqlite3OsLock(&pPager->fd, EXCLUSIVE_LOCK);
   }while( rc==SQLITE_BUSY && 

src/printf.c

@@ -810,7 +810,8 @@ void sqlite3DebugPrintf(const char *zFormat, ...){
   va_start(ap, zFormat);
   base_vprintf(0, 0, zBuf, sizeof(zBuf), zFormat, ap);
   va_end(ap);
-  fprintf(stderr,"%s", zBuf);
+  fprintf(stdout,"%s", zBuf);
+  fflush(stdout);
 }
 #endif
 

src/test1.c

@@ -13,7 +13,7 @@
 ** is not included in the SQLite library.  It is used for automated
 ** testing of the SQLite library.
 **
-** $Id: test1.c,v 1.69 2004/06/01 14:09:29 danielk1977 Exp $
+** $Id: test1.c,v 1.70 2004/06/07 16:27:46 drh Exp $
 */
 #include "sqliteInt.h"
 #include "tcl.h"
@@ -1913,6 +1913,7 @@ int Sqlitetest1_Init(Tcl_Interp *interp){
 
   };
   int i;
+  extern int sqlite3_os_trace;
 
   for(i=0; i<sizeof(aCmd)/sizeof(aCmd[0]); i++){
     Tcl_CreateCommand(interp, aCmd[i].zName, aCmd[i].xProc, 0, 0);
@@ -1929,6 +1930,8 @@ int Sqlitetest1_Init(Tcl_Interp *interp){
       (char*)&sqlite3_open_file_count, TCL_LINK_INT);
   Tcl_LinkVar(interp, "sqlite_current_time", 
       (char*)&sqlite3_current_time, TCL_LINK_INT);
+  Tcl_LinkVar(interp, "sqlite_os_trace",
+      (char*)&sqlite3_os_trace, TCL_LINK_INT);
   Tcl_LinkVar(interp, "sqlite_static_bind_value",
       (char*)&sqlite_static_bind_value, TCL_LINK_STRING);
   return TCL_OK;

src/vdbeaux.c

@@ -922,15 +922,25 @@ static int vdbeCommit(sqlite *db){
     }
   }
 
-  /* The simple case - if less than two databases have write-transactions
+  /* The simple case - no more than one database file (not counting the TEMP
-  ** active, there is no need for the master-journal.
+  ** database) has a transaction active.   There is no need for the
+  ** master-journal.
   */
-  if( nTrans<2 ){
+  if( nTrans<=1 ){
-    for(i=0; i<db->nDb; i++){ 
+    for(i=0; rc==SQLITE_OK && i<db->nDb; i++){ 
       Btree *pBt = db->aDb[i].pBt;
       if( pBt ){
-        int rc2 = sqlite3BtreeCommit(db->aDb[i].pBt);
+        rc = sqlite3BtreeSync(pBt, 0);
-        if( rc==SQLITE_OK ) rc = rc2;
+      }
+    }
+
+    /* Do the commit only if all databases successfully synced */
+    if( rc==SQLITE_OK ){
+      for(i=0; i<db->nDb; i++){
+        Btree *pBt = db->aDb[i].pBt;
+        if( pBt ){
+          sqlite3BtreeCommit(pBt);
+        }
       }
     }
   }
@@ -1036,7 +1046,7 @@ static int vdbeCommit(sqlite *db){
       }
     }
   }
-  return SQLITE_OK;
+  return rc;
 }
 
 /* 

test/attach2.test

@@ -12,15 +12,19 @@
 # focus of this script is testing the ATTACH and DETACH commands
 # and related functionality.
 #
-# $Id: attach2.test,v 1.11 2004/05/31 12:34:54 danielk1977 Exp $
+# $Id: attach2.test,v 1.12 2004/06/07 16:27:47 drh Exp $
 #
-
+set sqlite_os_trace 0
 
 set testdir [file dirname $argv0]
 source $testdir/tester.tcl
 
+
 # Ticket #354
 #
+# Databases test.db and test2.db contain identical schemas.  Make
+# sure we can attach test2.db from test.db.
+#
 do_test attach2-1.1 {
   db eval {
     CREATE TABLE t1(a,b);
@@ -49,40 +53,37 @@ proc db_list {db} {
 }
 db eval {DETACH t2}
 do_test attach2-2.1 {
-  # lock test2.db then try to attach it.  Should get an error.
+  # lock test2.db then try to attach it.  This is no longer an error because
+  # db2 just RESERVES the database.  It does not obtain a write-lock until
+  # we COMMIT.
   db2 eval {BEGIN}
   db2 eval {UPDATE t1 SET a = 0 WHERE 0}
   catchsql {
     ATTACH 'test2.db' AS t2;
   }
-} {1 {database is locked}}
-do_test attach2-2.2 {
-  # make sure test2.db did not get attached.
-  db_list db
-} {0 main 1 temp}
-do_test attach2-2.3 {
-  # unlock test2.db and try to attach again.  should work this time.
-  db2 eval {COMMIT}
-  catchsql {
-    ATTACH 'test2.db' AS t2;
-  }
 } {0 {}}
-do_test attach2-2.4 {
+do_test attach2-2.2 {
+  # make sure test2.db did get attached.
   db_list db
 } {0 main 1 temp 2 t2}
+db2 eval {COMMIT}
+
 do_test attach2-2.5 {
+  # Make sure we can read test2.db from db
   catchsql {
     SELECT name FROM t2.sqlite_master;
   }
 } {0 {t1 x1}}
 do_test attach2-2.6 {
-  # lock test2.db and try to read from it.  should get an error.
+  # lock test2.db and try to read from it.  This should still work because
+  # the lock is only a RESERVED lock which does not prevent reading.
+  #
   db2 eval BEGIN
   db2 eval {UPDATE t1 SET a = 0 WHERE 0}
   catchsql {
     SELECT name FROM t2.sqlite_master;
   }
-} {1 {database is locked}}
+} {0 {t1 x1}}
 do_test attach2-2.7 {
   # but we can still read from test1.db even though test2.db is locked.
   catchsql {
@@ -125,7 +126,7 @@ do_test attach2-2.12 {
   }
 } {1 {cannot commit - no transaction is active}}
 
-# Ticket #574:  Make sure it works usingi the non-callback API
+# Ticket #574:  Make sure it works using the non-callback API
 #
 do_test attach2-3.1 {
   db close
@@ -146,6 +147,7 @@ db close
 for {set i 2} {$i<=15} {incr i} {
   catch {db$i close}
 }
+set sqlite_os_trace 0
 
 # Tests attach2-4.* test that read-locks work correctly with attached
 # databases.
@@ -157,14 +159,17 @@ do_test attach2-4.1 {
 } {}
 
 do_test attach2-4.2 {
-  # Handle 'db' read-locks the main file
+  # Handle 'db' read-locks test.db
   execsql {BEGIN}
   execsql {SELECT * FROM t1}
 } {}
 do_test attach2-4.3 {
+  # The read lock held by db does not prevent db2 from reading test.db
   execsql {SELECT * FROM t1} db2
 } {}
 do_test attach2-4.4 {
+  # db is only holding a read lock on test.db, so we should not be able
+  # to commit a write to test.db from db2
   set r [catch { 
     execsql {
       INSERT INTO t1 VALUES(1, 2)
@@ -173,40 +178,39 @@ do_test attach2-4.4 {
   list $r $msg
 } {1 {database is locked}}
 do_test attach2-4.5 {
-  # Handle 'db2' write-locks file2
+  # Handle 'db2' reserves file2.
   execsql {BEGIN} db2
   execsql {INSERT INTO file2.t1 VALUES(1, 2)} db2
 } {}
-do_test attach2-4.6 {
+do_test attach2-4.6.1 {
-  set r [catch { 
+  # Reads are allowed against a reserved database.
-    execsql {
+  catchsql {
-      SELECT * FROM file2.t1;
+    SELECT * FROM file2.t1;
-    } 
+  }
-  } msg]
+} {0 {}}
-  list $r $msg
+do_test attach2-4.6.2 {
+  # Writes against a reserved database are not allowed.
+  catchsql {
+    UPDATE file2.t1 SET a=0;
+  }
 } {1 {database is locked}}
 do_test attach2-4.7 {
   # Ensure handle 'db' retains the lock on the main file after
-  # failing to obtain a read-lock on file2.
+  # failing to obtain a write-lock on file2.
-  set r [catch { 
+  catchsql {
-    execsql {
+    INSERT INTO t1 VALUES(1, 2)
-      INSERT INTO t1 VALUES(1, 2)
+  } db2 
-    } db2 
-  } msg]
-  list $r $msg
 } {1 {database is locked}}
 do_test attach2-4.8 {
-  # Read lock the main file with db2. Now both handles have a read lock
+  # Read lock the main file with db2. Now both db and db2 have a read lock
   # on the main file, db2 has a write-lock on file2.
   execsql {SELECT * FROM t1} db2
 } {}
 do_test attach2-4.9 {
   # Try to upgrade the handle 'db' lock.
-  set r [catch { 
+  catchsql {
-    execsql {
+    INSERT INTO t1 VALUES(1, 2)
-      INSERT INTO t1 VALUES(1, 2)
+  }
-    } 
-  } msg]
   list $r $msg
 } {1 {database is locked}}
 do_test attach2-4.10 {
@@ -232,8 +236,3 @@ db2 close
 file delete -force test2.db
 
 finish_test
-
-
-
-
-