Fixes to the OsUnlock() interface. Correctly leave a SHARED lock behind

when requested.  Honor the error code that OsUnlock() returns.
Ticket #913 and #938. (CVS 1997)

FossilOrigin-Name: c4697503d0ad080290b91e96dfc9a1a63f2df7e6

3 files changed, 51 insertions, 36 deletions

diff --git a/src/os_unix.c b/src/os_unix.c
index a5be702f2..88d35f9ce 100644
--- a/src/os_unix.c
+++ b/src/os_unix.c
@@ -1021,11 +1021,14 @@ end_lock:
 ** If the locking level of the file descriptor is already at or below
 ** the requested locking level, this routine is a no-op.
 **
-** It is not possible for this routine to fail.
+** It is not possible for this routine to fail if the second argument
+** is NO_LOCK.  If the second argument is SHARED_LOCK, this routine
+** might return SQLITE_IOERR instead of SQLITE_OK.
 */
 int sqlite3OsUnlock(OsFile *id, int locktype){
   struct lockInfo *pLock;
   struct flock lock;
+  int rc = SQLITE_OK;
 
   assert( id->isOpen );
   TRACE7("UNLOCK %d %d was %d(%d,%d) pid=%d\n", id->h, locktype, id->locktype, 
@@ -1040,11 +1043,16 @@ int sqlite3OsUnlock(OsFile *id, int locktype){
   assert( pLock->cnt!=0 );
   if( id->locktype>SHARED_LOCK ){
     assert( pLock->locktype==id->locktype );
-    lock.l_type = F_RDLCK;
-    lock.l_whence = SEEK_SET;
-    lock.l_start = SHARED_FIRST;
-    lock.l_len = SHARED_SIZE;
-    fcntl(id->h, F_SETLK, &lock);
+    if( locktype==SHARED_LOCK ){
+      lock.l_type = F_RDLCK;
+      lock.l_whence = SEEK_SET;
+      lock.l_start = SHARED_FIRST;
+      lock.l_len = SHARED_SIZE;
+      if( fcntl(id->h, F_SETLK, &lock)!=0 ){
+        /* This should never happen */
+        rc = SQLITE_IOERR;
+      }
+    }
     lock.l_type = F_UNLCK;
     lock.l_whence = SEEK_SET;
     lock.l_start = PENDING_BYTE;
@@ -1087,7 +1095,7 @@ int sqlite3OsUnlock(OsFile *id, int locktype){
   }
   sqlite3OsLeaveMutex();
   id->locktype = locktype;
-  return SQLITE_OK;
+  return rc;
 }
 
 /*
diff --git a/src/os_win.c b/src/os_win.c
index c79ae6285..f6e3e3ea8 100644
--- a/src/os_win.c
+++ b/src/os_win.c
@@ -375,20 +375,23 @@ static int isNT(void){
 }
 
 /*
-** Acquire a reader lock on the range of bytes from iByte...iByte+nByte-1.
+** Acquire a reader lock.
 ** Different API routines are called depending on whether or not this
 ** is Win95 or WinNT.
 */
-static int getReadLock(HANDLE h, unsigned int iByte, unsigned int nByte){
+static int getReadLock(OsFile *id){
   int res;
   if( isNT() ){
     OVERLAPPED ovlp;
-    ovlp.Offset = iByte;
+    ovlp.Offset = SHARED_FIRST;
     ovlp.OffsetHigh = 0;
     ovlp.hEvent = 0;
-    res = LockFileEx(h, LOCKFILE_FAIL_IMMEDIATELY, 0, nByte, 0, &ovlp);
+    res = LockFileEx(id->h, LOCKFILE_FAIL_IMMEDIATELY, 0, SHARED_SIZE,0,&ovlp);
   }else{
-    res = LockFile(h, iByte, 0, nByte, 0);
+    int lk;
+    sqlite3Randomness(sizeof(lk), &lk);
+    id->sharedLockByte = (lk & 0x7fffffff)%(SHARED_SIZE - 1);
+    res = LockFile(id->h, SHARED_FIRST+id->sharedLockByte, 0, 1, 0);
   }
   return res;
 }
@@ -479,14 +482,7 @@ int sqlite3OsLock(OsFile *id, int locktype){
   */
   if( locktype==SHARED_LOCK && res ){
     assert( id->locktype==NO_LOCK );
-    if( isNT() ){
-      res = getReadLock(id->h, SHARED_FIRST, SHARED_SIZE);
-    }else{
-      int lk;
-      sqlite3Randomness(sizeof(lk), &lk);
-      id->sharedLockByte = (lk & 0x7fffffff)%(SHARED_SIZE - 1);
-      res = LockFile(id->h, SHARED_FIRST+id->sharedLockByte, 0, 1, 0);
-    }
+    res = getReadLock(id);
     if( res ){
       newLocktype = SHARED_LOCK;
     }
@@ -573,10 +569,13 @@ int sqlite3OsCheckReservedLock(OsFile *id){
 ** If the locking level of the file descriptor is already at or below
 ** the requested locking level, this routine is a no-op.
 **
-** It is not possible for this routine to fail.
+** It is not possible for this routine to fail if the second argument
+** is NO_LOCK.  If the second argument is SHARED_LOCK then this routine
+** might return SQLITE_IOERR;
 */
 int sqlite3OsUnlock(OsFile *id, int locktype){
-  int rc, type;
+  int type;
+  int rc = SQLITE_OK;
   assert( id->isOpen );
   assert( locktype<=SHARED_LOCK );
   TRACE5("UNLOCK %d to %d was %d(%d)\n", id->h, locktype,
@@ -584,18 +583,23 @@ int sqlite3OsUnlock(OsFile *id, int locktype){
   type = id->locktype;
   if( type>=EXCLUSIVE_LOCK ){
     UnlockFile(id->h, SHARED_FIRST, 0, SHARED_SIZE, 0);
+    if( locktype==SHARED_LOCK && !getReadLock(id) ){
+      /* This should never happen.  We should always be able to
+      ** reacquire the read lock */
+      rc = SQLITE_IOERR;
+    }
   }
   if( type>=RESERVED_LOCK ){
     UnlockFile(id->h, RESERVED_BYTE, 0, 1, 0);
   }
-  if( locktype==NO_LOCK && type>=SHARED_LOCK && type<EXCLUSIVE_LOCK ){
+  if( locktype==NO_LOCK && type>=SHARED_LOCK ){
     unlockReadLock(id);
   }
   if( type>=PENDING_LOCK ){
     UnlockFile(id->h, PENDING_BYTE, 0, 1, 0);
   }
   id->locktype = locktype;
-  return SQLITE_OK;
+  return rc;
 }
 
 /*
diff --git a/src/pager.c b/src/pager.c
index d62c490d9..8b20c16c5 100644
--- a/src/pager.c
+++ b/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.165 2004/10/01 02:00:31 drh Exp $
+** @(#) $Id: pager.c,v 1.166 2004/10/02 20:38:28 drh Exp $
 */
 #include "sqliteInt.h"
 #include "os.h"
@@ -755,6 +755,7 @@ static void pager_reset(Pager *pPager){
 */
 static int pager_unwritelock(Pager *pPager){
   PgHdr *pPg;
+  int rc;
   assert( !pPager->memDb );
   if( pPager->state<PAGER_RESERVED ){
     return SQLITE_OK;
@@ -780,11 +781,11 @@ static int pager_unwritelock(Pager *pPager){
   }else{
     assert( pPager->dirtyCache==0 || pPager->useJournal==0 );
   }
-  sqlite3OsUnlock(&pPager->fd, SHARED_LOCK);
+  rc = sqlite3OsUnlock(&pPager->fd, SHARED_LOCK);
   pPager->state = PAGER_SHARED;
   pPager->origDbSize = 0;
   pPager->setMaster = 0;
-  return SQLITE_OK;
+  return rc;
 }
 
 /*
@@ -2342,20 +2343,15 @@ static int pager_open_journal(Pager *pPager){
   sqlite3pager_pagecount(pPager);
   pPager->aInJournal = sqliteMalloc( pPager->dbSize/8 + 1 );
   if( pPager->aInJournal==0 ){
-    sqlite3OsUnlock(&pPager->fd, SHARED_LOCK);
-    pPager->state = PAGER_SHARED;
-    return SQLITE_NOMEM;
+    rc = SQLITE_NOMEM;
+    goto failed_to_open_journal;
   }
   rc = sqlite3OsOpenExclusive(pPager->zJournal, &pPager->jfd,pPager->tempFile);
   pPager->journalOff = 0;
   pPager->setMaster = 0;
   pPager->journalHdr = 0;
   if( rc!=SQLITE_OK ){
-    sqliteFree(pPager->aInJournal);
-    pPager->aInJournal = 0;
-    sqlite3OsUnlock(&pPager->fd, SHARED_LOCK);
-    pPager->state = PAGER_SHARED;
-    return rc;
+    goto failed_to_open_journal;
   }
   sqlite3OsOpenDirectory(pPager->zDirectory, &pPager->jfd);
   pPager->journalOpen = 1;
@@ -2380,7 +2376,14 @@ static int pager_open_journal(Pager *pPager){
       rc = SQLITE_FULL;
     }
   }
-  return rc;  
+  return rc;
+
+failed_to_open_journal:
+  sqliteFree(pPager->aInJournal);
+  pPager->aInJournal = 0;
+  sqlite3OsUnlock(&pPager->fd, NO_LOCK);
+  pPager->state = PAGER_UNLOCK;
+  return rc;
 }
 
 /*