From: dan <dan@noemail.net>
Date: Wed, 1 Nov 2017 20:59:28 +0000 (+0000)
Subject: If a readonly_shm connection cannot map the *-shm file because no other
X-Git-Tag: version-3.22.0~199^2~28
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=92c02da33ec2717828eba13400355d43a7a6395f;p=thirdparty%2Fsqlite.git

If a readonly_shm connection cannot map the *-shm file because no other
process is holding the DMS lock, have it read from the database file only,
ignoring any content in the wal file.

FossilOrigin-Name: ce5d13c2de69b73378637d4f7e109714f7cd17bf1d1ad995e0be442d517ed1b3
---

diff --git a/manifest b/manifest
index a6e4cc1fc1..45c7b16d67 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Merge\slatest\strunk\schanges\sinto\sthis\sbranch.
-D 2017-11-01T07:06:41.244
+C If\sa\sreadonly_shm\sconnection\scannot\smap\sthe\s*-shm\sfile\sbecause\sno\sother\nprocess\sis\sholding\sthe\sDMS\slock,\shave\sit\sread\sfrom\sthe\sdatabase\sfile\sonly,\nignoring\sany\scontent\sin\sthe\swal\sfile.
+D 2017-11-01T20:59:28.295
 F Makefile.in 5bae3f2f3d42f2ad52b141562d74872c97ac0fca6c54953c91bb150a0e6427a8
 F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434
 F Makefile.msc 3a5cb477ec3ce5274663b693164e349db63348667cd45bad78cc13d580b691e2
@@ -447,7 +447,7 @@ F src/os.c 22d31db3ca5a96a408fbf1ceeaaebcaf64c87024d2ff9fe1cf2ddbec3e75c104
 F src/os.h 48388821692e87da174ea198bf96b1b2d9d83be5dfc908f673ee21fafbe0d432
 F src/os_common.h b2f4707a603e36811d9b1a13278bffd757857b85
 F src/os_setup.h 0dbaea40a7d36bf311613d31342e0b99e2536586
-F src/os_unix.c 9137cfdb42e83f4fb599aa2b1c6996f368aacbb410bde53b534e766a61ba65ca
+F src/os_unix.c e376adf6014df7d1a73faaaa6c87e6eb9b299b157a58cccff02fad8abc943fe7
 F src/os_win.c 6892c3ff23b7886577e47f13d827ca220c0831bae3ce00eea8c258352692f8c6
 F src/os_win.h 7b073010f1451abe501be30d12f6bc599824944a
 F src/pager.c 07cf850241667874fcce9d7d924c814305e499b26c804322e2261247b5921903
@@ -543,7 +543,7 @@ F src/vdbesort.c 731a09e5cb9e96b70c394c1b7cf3860fbe84acca7682e178615eb941a3a0ef2
 F src/vdbetrace.c 48e11ebe040c6b41d146abed2602e3d00d621d7ebe4eb29b0a0f1617fd3c2f6c
 F src/vtab.c 0e4885495172e1bdf54b12cce23b395ac74ef5729031f15e1bc1e3e6b360ed1a
 F src/vxworks.h d2988f4e5a61a4dfe82c6524dd3d6e4f2ce3cdb9
-F src/wal.c cc9b1120f1955b66af425630c9893acd537a39d967fd39d404417f0a1b4c1579
+F src/wal.c 1521bdcfe9a536752a339c91b63ca0d226d8d266636391aac93537fdea8657fc
 F src/wal.h 8de5d2d3de0956d6f6cb48c83a4012d5f227b8fe940f3a349a4b7e85ebcb492a
 F src/walker.c d591e8a9ccf60abb010966b354fcea4aa08eba4d83675c2b281a8764c76cc22f
 F src/where.c b7a075f5fb3d912a891dcc3257f538372bb4a1622dd8ca7d752ad95ce8949ba4
@@ -1527,6 +1527,7 @@ F test/waloverwrite.test dad2f26567f1b45174e54fbf9a8dc1cb876a7f03
 F test/walpersist.test 8c6b7e3ec1ba91b5e4dc4e0921d6d3f87cd356a6
 F test/walprotocol.test 0b92feb132ccebd855494d917d3f6c2d717ace20
 F test/walro.test e492598baa8cd7777fef6203f6fe922c20cd691cc19e60ccd0dd0dbc68394d0a
+F test/walro2.test e2cd102cafceafaf19c56d55e55222fdd26d7621d7ef42b0eaf35c06c5bb3d19
 F test/walshared.test 0befc811dcf0b287efae21612304d15576e35417
 F test/walslow.test c05c68d4dc2700a982f89133ce103a1a84cc285f
 F test/walthread.test de8dbaf6d9e41481c460ba31ca61e163d7348f8e
@@ -1667,7 +1668,7 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P d655bfabd110999b6808073c334869c5b6a8334df56811df883e47e56d3f1cbb bb39744f4b2b25c10d293e85db7579e2a99c639fdab45e93d1de75952b68b2de
-R 46264ab14606a934f99609a277e3ff11
+P 985bfc992950625a45a7521bf4c8438cd0170de974dff976968be158ac5922a9
+R fcc128b567671c56015745eca2c28340
 U dan
-Z 7bfe9089058e9a41a8fafe2fd8ba1cb5
+Z e9ea9ea441f189aa52edada39fcf2f83
diff --git a/manifest.uuid b/manifest.uuid
index 48096a84ae..ebf42c65a3 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-985bfc992950625a45a7521bf4c8438cd0170de974dff976968be158ac5922a9
\ No newline at end of file
+ce5d13c2de69b73378637d4f7e109714f7cd17bf1d1ad995e0be442d517ed1b3
\ No newline at end of file
diff --git a/src/os_unix.c b/src/os_unix.c
index b94d417f49..e90e335cb7 100644
--- a/src/os_unix.c
+++ b/src/os_unix.c
@@ -4108,6 +4108,7 @@ struct unixShmNode {
   int szRegion;              /* Size of shared-memory regions */
   u16 nRegion;               /* Size of array apRegion */
   u8 isReadonly;             /* True if read-only */
+  u8 isUnlocked;             /* True if no DMS lock held */
   char **apRegion;           /* Array of mapped shared-memory regions */
   int nRef;                  /* Number of unixShm objects pointing to this */
   unixShm *pFirst;           /* All unixShm objects pointing to this */
@@ -4270,6 +4271,64 @@ static void unixShmPurge(unixFile *pFd){
   }
 }
 
+/*
+** The DMS lock has not yet been taken on shm file pShmNode. Attempt to
+** take it now. Return SQLITE_OK if successful, or an SQLite error
+** code otherwise.
+**
+** If the DMS cannot be locked because this is a readonly_shm=1 
+** connection and no other process already holds a lock, return
+** SQLITE_READONLY_CANTLOCK and set pShmNode->isUnlocked=1.
+*/
+static int unixLockSharedMemory(unixFile *pDbFd, unixShmNode *pShmNode){
+  struct flock lock;
+  int rc = SQLITE_OK;
+
+  /* Use F_GETLK to determine the locks other processes are holding
+  ** on the DMS byte. If it indicates that another process is holding
+  ** a SHARED lock, then this process may also take a SHARED lock
+  ** and proceed with opening the *-shm file. 
+  **
+  ** Or, if no other process is holding any lock, then this process
+  ** is the first to open it. In this case take an EXCLUSIVE lock on the
+  ** DMS byte and truncate the *-shm file to zero bytes in size. Then
+  ** downgrade to a SHARED lock on the DMS byte.
+  **
+  ** If another process is holding an EXCLUSIVE lock on the DMS byte,
+  ** return SQLITE_BUSY to the caller (it will try again). An earlier
+  ** version of this code attempted the SHARED lock at this point. But
+  ** this introduced a subtle race condition: if the process holding
+  ** EXCLUSIVE failed just before truncating the *-shm file, then this
+  ** process might open and use the *-shm file without truncating it.
+  ** And if the *-shm file has been corrupted by a power failure or
+  ** system crash, the database itself may also become corrupt.  */
+  lock.l_whence = SEEK_SET;
+  lock.l_start = UNIX_SHM_DMS;
+  lock.l_len = 1;
+  lock.l_type = F_WRLCK;
+  if( osFcntl(pShmNode->h, F_GETLK, &lock)!=0 ) {
+    rc = SQLITE_IOERR_LOCK;
+  }else if( lock.l_type==F_UNLCK ){
+    if( pShmNode->isReadonly ){
+      pShmNode->isUnlocked = 1;
+      rc = SQLITE_READONLY_CANTLOCK;
+    }else{
+      rc = unixShmSystemLock(pDbFd, F_WRLCK, UNIX_SHM_DMS, 1);
+      if( rc==SQLITE_OK && robust_ftruncate(pShmNode->h, 0) ){
+        rc = unixLogError(SQLITE_IOERR_SHMOPEN,"ftruncate",pShmNode->zFilename);
+      }
+    }
+  }else if( lock.l_type==F_WRLCK ){
+    rc = SQLITE_BUSY;
+  }
+
+  if( rc==SQLITE_OK ){
+    assert( lock.l_type==F_UNLCK || lock.l_type==F_RDLCK );
+    rc = unixShmSystemLock(pDbFd, F_RDLCK, UNIX_SHM_DMS, 1);
+  }
+  return rc;
+}
+
 /*
 ** Open a shared-memory area associated with open database file pDbFd.  
 ** This particular implementation uses mmapped files.
@@ -4308,7 +4367,7 @@ static void unixShmPurge(unixFile *pFd){
 static int unixOpenSharedMemory(unixFile *pDbFd){
   struct unixShm *p = 0;          /* The connection to be opened */
   struct unixShmNode *pShmNode;   /* The underlying mmapped file */
-  int rc;                         /* Result code */
+  int rc = SQLITE_OK;             /* Result code */
   unixInodeInfo *pInode;          /* The inode of fd */
   char *zShmFilename;             /* Name of the file used for SHM */
   int nShmFilename;               /* Size of the SHM filename in bytes */
@@ -4372,7 +4431,6 @@ static int unixOpenSharedMemory(unixFile *pDbFd){
     }
 
     if( pInode->bProcessLock==0 ){
-      struct flock lock;
       int openFlags = O_RDWR | O_CREAT;
       if( sqlite3_uri_boolean(pDbFd->zPath, "readonly_shm", 0) ){
         openFlags = O_RDONLY;
@@ -4389,50 +4447,9 @@ static int unixOpenSharedMemory(unixFile *pDbFd){
       ** the original owner will not be able to connect.
       */
       robustFchown(pShmNode->h, sStat.st_uid, sStat.st_gid);
-  
-      /* Use F_GETLK to determine the locks other processes are holding
-      ** on the DMS byte. If it indicates that another process is holding
-      ** a SHARED lock, then this process may also take a SHARED lock
-      ** and proceed with opening the *-shm file. 
-      **
-      ** Or, if no other process is holding any lock, then this process
-      ** is the first to open it. In this case take an EXCLUSIVE lock on the
-      ** DMS byte and truncate the *-shm file to zero bytes in size. Then
-      ** downgrade to a SHARED lock on the DMS byte.
-      **
-      ** If another process is holding an EXCLUSIVE lock on the DMS byte,
-      ** return SQLITE_BUSY to the caller (it will try again). An earlier
-      ** version of this code attempted the SHARED lock at this point. But
-      ** this introduced a subtle race condition: if the process holding
-      ** EXCLUSIVE failed just before truncating the *-shm file, then this
-      ** process might open and use the *-shm file without truncating it.
-      ** And if the *-shm file has been corrupted by a power failure or
-      ** system crash, the database itself may also become corrupt.  */
-      rc = SQLITE_OK;
-      lock.l_whence = SEEK_SET;
-      lock.l_start = UNIX_SHM_DMS;
-      lock.l_len = 1;
-      lock.l_type = F_WRLCK;
-      if( osFcntl(pShmNode->h, F_GETLK, &lock)!=0 ) {
-        rc = SQLITE_IOERR_LOCK;
-      }else if( lock.l_type==F_UNLCK ){
-        if( pShmNode->isReadonly ){
-          rc = SQLITE_CANTOPEN_DIRTYWAL;
-        }else{
-          rc = unixShmSystemLock(pDbFd, F_WRLCK, UNIX_SHM_DMS, 1);
-          if( rc==SQLITE_OK && robust_ftruncate(pShmNode->h, 0) ){
-            rc = unixLogError(SQLITE_IOERR_SHMOPEN, "ftruncate", zShmFilename);
-          }
-        }
-      }else if( lock.l_type==F_WRLCK ){
-        rc = SQLITE_BUSY;
-      }
 
-      if( rc==SQLITE_OK ){
-        assert( lock.l_type==F_UNLCK || lock.l_type==F_RDLCK );
-        rc = unixShmSystemLock(pDbFd, F_RDLCK, UNIX_SHM_DMS, 1);
-      }
-      if( rc ) goto shm_open_err;
+      rc = unixLockSharedMemory(pDbFd, pShmNode);
+      if( rc!=SQLITE_OK && rc!=SQLITE_READONLY_CANTLOCK ) goto shm_open_err;
     }
   }
 
@@ -4456,7 +4473,7 @@ static int unixOpenSharedMemory(unixFile *pDbFd){
   p->pNext = pShmNode->pFirst;
   pShmNode->pFirst = p;
   sqlite3_mutex_leave(pShmNode->mutex);
-  return SQLITE_OK;
+  return rc;
 
   /* Jump here on any error */
 shm_open_err:
@@ -4508,6 +4525,11 @@ static int unixShmMap(
   p = pDbFd->pShm;
   pShmNode = p->pShmNode;
   sqlite3_mutex_enter(pShmNode->mutex);
+  if( pShmNode->isUnlocked ){
+    rc = unixLockSharedMemory(pDbFd, pShmNode);
+    if( rc!=SQLITE_OK ) goto shmpage_out;
+    pShmNode->isUnlocked = 0;
+  }
   assert( szRegion==pShmNode->szRegion || pShmNode->nRegion==0 );
   assert( pShmNode->pInode==pDbFd->pInode );
   assert( pShmNode->h>=0 || pDbFd->pInode->bProcessLock==1 );
diff --git a/src/wal.c b/src/wal.c
index 19c9ea0a08..1a11eb31e0 100644
--- a/src/wal.c
+++ b/src/wal.c
@@ -575,9 +575,11 @@ static int walIndexPage(Wal *pWal, int iPage, volatile u32 **ppPage){
       rc = sqlite3OsShmMap(pWal->pDbFd, iPage, WALINDEX_PGSZ, 
           pWal->writeLock, (void volatile **)&pWal->apWiData[iPage]
       );
-      if( rc==SQLITE_READONLY ){
+      if( (rc&0xff)==SQLITE_READONLY ){
         pWal->readOnly |= WAL_SHM_RDONLY;
-        rc = SQLITE_OK;
+        if( rc==SQLITE_READONLY ){
+          rc = SQLITE_OK;
+        }
       }
     }
   }
@@ -2084,6 +2086,14 @@ static int walIndexReadHdr(Wal *pWal, int *pChanged){
   assert( pChanged );
   rc = walIndexPage(pWal, 0, &page0);
   if( rc!=SQLITE_OK ){
+    if( rc==SQLITE_READONLY_CANTLOCK 
+#ifdef SQLITE_ENABLE_SNAPSHOT
+        && pWal->pSnapshot==0 
+#endif
+    ){
+      memset(&pWal->hdr, 0, sizeof(WalIndexHdr));
+      rc = SQLITE_OK;
+    }
     return rc;
   };
   assert( page0 || pWal->writeLock==0 );
@@ -2259,8 +2269,10 @@ static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int cnt){
     }
   }
 
-  pInfo = walCkptInfo(pWal);
-  if( !useWal && pInfo->nBackfill==pWal->hdr.mxFrame 
+  assert( pWal->nWiData>0 );
+  assert( pWal->apWiData[0] || (pWal->readOnly & WAL_SHM_RDONLY) );
+  pInfo = pWal->apWiData[0] ? walCkptInfo(pWal) : 0;
+  if( !useWal && (pInfo==0 || pInfo->nBackfill==pWal->hdr.mxFrame)
 #ifdef SQLITE_ENABLE_SNAPSHOT
    && (pWal->pSnapshot==0 || pWal->hdr.mxFrame==0
      || 0==memcmp(&pWal->hdr, pWal->pSnapshot, sizeof(WalIndexHdr)))
@@ -2272,7 +2284,9 @@ static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int cnt){
     rc = walLockShared(pWal, WAL_READ_LOCK(0));
     walShmBarrier(pWal);
     if( rc==SQLITE_OK ){
-      if( memcmp((void *)walIndexHdr(pWal), &pWal->hdr, sizeof(WalIndexHdr)) ){
+      if( pInfo
+       && memcmp((void *)walIndexHdr(pWal), &pWal->hdr, sizeof(WalIndexHdr)) 
+      ){
         /* It is not safe to allow the reader to continue here if frames
         ** may have been appended to the log before READ_LOCK(0) was obtained.
         ** When holding READ_LOCK(0), the reader ignores the entire log file,
diff --git a/test/walro2.test b/test/walro2.test
new file mode 100644
index 0000000000..2337776f27
--- /dev/null
+++ b/test/walro2.test
@@ -0,0 +1,87 @@
+# 2011 May 09
+#
+# 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 contains tests for using WAL databases in read-only mode.
+#
+
+set testdir [file dirname $argv0]
+source $testdir/tester.tcl
+source $testdir/lock_common.tcl
+set ::testprefix walro
+
+# These tests are only going to work on unix.
+#
+if {$::tcl_platform(platform) != "unix"} {
+  finish_test
+  return
+}
+
+# And only if the build is WAL-capable.
+#
+ifcapable !wal {
+  finish_test
+  return
+}
+
+do_multiclient_test tn {
+  
+  # Close all connections and delete the database.
+  #
+  code1 { db close  }
+  code2 { db2 close }
+  code3 { db3 close }
+  forcedelete test.db
+  
+  # Do not run tests with the connections in the same process.
+  #
+  if {$tn==2} continue
+
+  foreach c {code1 code2 code3} {
+    $c {
+      sqlite3_shutdown
+      sqlite3_config_uri 1
+    }
+  }
+
+  do_test 1.1 {
+    code2 { sqlite3 db2 test.db }
+    sql2 { 
+      CREATE TABLE t1(x, y);
+      PRAGMA journal_mode = WAL;
+      INSERT INTO t1 VALUES('a', 'b');
+      INSERT INTO t1 VALUES('c', 'd');
+    }
+    file exists test.db-shm
+  } {1}
+
+  do_test 1.2 {
+    forcecopy test.db test.db2
+    forcecopy test.db-wal test.db2-wal
+    forcecopy test.db-shm test.db2-shm
+    code1 {
+      sqlite3 db file:test.db2?readonly_shm=1
+    }
+
+    sql1 { SELECT * FROM t1 }
+  } {}
+
+  do_test 1.3.1 {
+    code3 { sqlite3 db3 test.db2 }
+    sql3 { SELECT * FROM t1 }
+  } {a b c d}
+
+  do_test 1.3.2 {
+    sql1 { SELECT * FROM t1 }
+  } {a b c d}
+
+}
+
+finish_test