From 9181ae990a3b8623d27813209e6649bf76565545 Mon Sep 17 00:00:00 2001 From: dan Date: Thu, 26 Oct 2017 17:05:22 +0000 Subject: [PATCH] Instead of extra locks, use F_GETLK to ensure that readonly_shm clients cannot connect to a wal-mode database if there are no writers. FossilOrigin-Name: 5492f457dc7cc5c416de4b4e61e84bd2f10b4e6ce54011b7a60feb47f629c923 --- manifest | 17 ++++---- manifest.uuid | 2 +- src/os_unix.c | 107 ++++++++++++++++++++++---------------------------- 3 files changed, 56 insertions(+), 70 deletions(-) diff --git a/manifest b/manifest index bf7be72e5e..c648e5a856 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Use\sextra\slocks\sto\sprevent\sa\sreadonly_shm=1\sprocess\sfrom\sconnecting\sto\sa\nWAL-mode\sdatabase\sif\sthere\sare\sno\swriters. -D 2017-10-25T23:28:13.771 +C Instead\sof\sextra\slocks,\suse\sF_GETLK\sto\sensure\sthat\sreadonly_shm\sclients\scannot\nconnect\sto\sa\swal-mode\sdatabase\sif\sthere\sare\sno\swriters. +D 2017-10-26T17:05:22.656 F Makefile.in e016061b23e60ac9ec27c65cb577292b6bde0307ca55abd874ab3487b3b1beb2 F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434 F Makefile.msc 37740aba9c4bb359c627eadccf1cfd7be4f5f847078723777ea7763969e533b1 @@ -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 bec568f6f89e57b987f5ca38ae90087687d8e18669cd37eceb48ca6b56a8fbfb +F src/os_unix.c 8103f60342c65d501b4e58b381796648d6584b4814ffee79cd3a6e0c12fb6545 F src/os_win.c 6892c3ff23b7886577e47f13d827ca220c0831bae3ce00eea8c258352692f8c6 F src/os_win.h 7b073010f1451abe501be30d12f6bc599824944a F src/pager.c 07cf850241667874fcce9d7d924c814305e499b26c804322e2261247b5921903 @@ -1666,10 +1666,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 d4f893e1ae53a0445939ea2920af87d21dd36270494381028b2eaebe5c188f18 -R 7c17d54c5cff309a6f13f49dd620b485 -T *branch * readonly-wal-recovery -T *sym-readonly-wal-recovery * -T -sym-trunk * -U drh -Z 26f9af4fe6668c97b92039b43092c035 +P 35d979082b4ab36d6a8975f8f15a50e69f46b72a173164d2b353377b9f758bd8 +R 87551e9df4f45e3704d812374bd0d192 +U dan +Z a113fb74c5589c985f30184211e03e42 diff --git a/manifest.uuid b/manifest.uuid index 44570bf71c..1815e9a333 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -35d979082b4ab36d6a8975f8f15a50e69f46b72a173164d2b353377b9f758bd8 \ No newline at end of file +5492f457dc7cc5c416de4b4e61e84bd2f10b4e6ce54011b7a60feb47f629c923 \ No newline at end of file diff --git a/src/os_unix.c b/src/os_unix.c index dadfaf9908..f689ff75fa 100644 --- a/src/os_unix.c +++ b/src/os_unix.c @@ -4145,7 +4145,6 @@ struct unixShm { */ #define UNIX_SHM_BASE ((22+SQLITE_SHM_NLOCK)*4) /* first lock byte */ #define UNIX_SHM_DMS (UNIX_SHM_BASE+SQLITE_SHM_NLOCK) /* deadman switch */ -#define UNIX_SHM_N_DMS 1000000000 /* Size of the DMS */ /* ** Apply posix advisory locks for all bytes from ofst through ofst+n-1. @@ -4167,6 +4166,12 @@ static int unixShmSystemLock( pShmNode = pFile->pInode->pShmNode; assert( sqlite3_mutex_held(pShmNode->mutex) || pShmNode->nRef==0 ); + /* Shared locks never span more than one byte */ + assert( n==1 || lockType!=F_RDLCK ); + + /* Locks are within range */ + assert( n>=1 && n<=SQLITE_SHM_NLOCK ); + if( pShmNode->h>=0 ){ /* Initialize the locking parameters */ memset(&f, 0, sizeof(f)); @@ -4181,44 +4186,36 @@ static int unixShmSystemLock( /* Update the global lock state and do debug tracing */ #ifdef SQLITE_DEBUG - if( ofst=1 && n<=SQLITE_SHM_NLOCK ); - - OSTRACE(("SHM-LOCK ")); - mask = ofst>31 ? 0xffff : (1<<(ofst+n)) - (1<exclMask &= ~mask; - pShmNode->sharedMask &= ~mask; - }else if( lockType==F_RDLCK ){ - OSTRACE(("read-lock %d ok", ofst)); - pShmNode->exclMask &= ~mask; - pShmNode->sharedMask |= mask; - }else{ - assert( lockType==F_WRLCK ); - OSTRACE(("write-lock %d ok", ofst)); - pShmNode->exclMask |= mask; - pShmNode->sharedMask &= ~mask; - } + { u16 mask; + OSTRACE(("SHM-LOCK ")); + mask = ofst>31 ? 0xffff : (1<<(ofst+n)) - (1<exclMask &= ~mask; + pShmNode->sharedMask &= ~mask; + }else if( lockType==F_RDLCK ){ + OSTRACE(("read-lock %d ok", ofst)); + pShmNode->exclMask &= ~mask; + pShmNode->sharedMask |= mask; }else{ - if( lockType==F_UNLCK ){ - OSTRACE(("unlock %d failed", ofst)); - }else if( lockType==F_RDLCK ){ - OSTRACE(("read-lock failed")); - }else{ - assert( lockType==F_WRLCK ); - OSTRACE(("write-lock %d failed", ofst)); - } + assert( lockType==F_WRLCK ); + OSTRACE(("write-lock %d ok", ofst)); + pShmNode->exclMask |= mask; + pShmNode->sharedMask &= ~mask; } - OSTRACE((" - afterwards %03x,%03x\n", - pShmNode->sharedMask, pShmNode->exclMask)); + }else{ + if( lockType==F_UNLCK ){ + OSTRACE(("unlock %d failed", ofst)); + }else if( lockType==F_RDLCK ){ + OSTRACE(("read-lock failed")); + }else{ + assert( lockType==F_WRLCK ); + OSTRACE(("write-lock %d failed", ofst)); + } + } + OSTRACE((" - afterwards %03x,%03x\n", + pShmNode->sharedMask, pShmNode->exclMask)); } #endif @@ -4392,38 +4389,30 @@ static int unixOpenSharedMemory(unixFile *pDbFd){ */ robustFchown(pShmNode->h, sStat.st_uid, sStat.st_gid); - /* Do not allow a read-only process to connect if there are no - ** writers, because a read-only process is unable to recover the - ** shm file following a system crash. - */ + /* Check to see if another process is holding the dead-man switch. + ** For a readonly_shm client, if no other process holds the DMS lock, + ** the file cannot be opened and SQLITE_CANTOPEN_DIRTYWAL is returned. + ** Or, for a read-write connection, if no other process holds a + ** DMS lock the file is truncated to zero bytes in size. */ rc = SQLITE_OK; if( pShmNode->isReadonly ){ - if( !unixShmSystemLock(pDbFd, F_RDLCK, UNIX_SHM_DMS, UNIX_SHM_N_DMS) ){ + struct flock lock; + 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, &lockInfo)!=0 ) { + rc = SQLITE_IOERR_LOCK; + }else if( lock.l_type==F_UNLCK ){ rc = SQLITE_CANTOPEN_DIRTYWAL; } - } - - /* If we are able to grab the dead-man switch, that means this is the - ** first (write-enable) process to connect to the database. In that - ** case, truncate the shm file because the contents found on disk might - ** be invalid leftovers from a system crash. The shm will be rebuilt - */ - if( unixShmSystemLock(pDbFd, F_WRLCK, UNIX_SHM_DMS, 1)==SQLITE_OK ){ + }else if( unixShmSystemLock(pDbFd, F_WRLCK, UNIX_SHM_DMS, 1)==SQLITE_OK ){ if( robust_ftruncate(pShmNode->h, 0) ){ rc = unixLogError(SQLITE_IOERR_SHMOPEN, "ftruncate", zShmFilename); } } - - /* Acquires locks to tell other processes that a this process is - ** running and therefore the shm is valid they do not need to run - ** recovery. - */ if( rc==SQLITE_OK ){ - unsigned r; rc = unixShmSystemLock(pDbFd, F_RDLCK, UNIX_SHM_DMS, 1); - sqlite3_randomness(sizeof(r), &r); - r = 1 + (r%(UNIX_SHM_N_DMS-1)); - (void)unixShmSystemLock(pDbFd, F_WRLCK, UNIX_SHM_DMS+r, 1); } if( rc ) goto shm_open_err; }