From 8337da6678bc2f83c0ba84aec89c751cdeadf1e5 Mon Sep 17 00:00:00 2001 From: dan Date: Fri, 28 Aug 2020 19:27:15 +0000 Subject: [PATCH 1/2] Modify the unixShmLock() function to avoid iterating through the (possibly large) set of connections to the same database file. FossilOrigin-Name: e0faddf0dfc3a40b6b94408296dd781dd0264ecc9f2129ce4405438433fb00e0 --- manifest | 15 ++++--- manifest.uuid | 2 +- src/os_unix.c | 111 +++++++++++++++++++++++++++++++------------------- 3 files changed, 80 insertions(+), 48 deletions(-) diff --git a/manifest b/manifest index abe12971e2..fd5ff92eb8 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\sfts5\stest\sto\sconfirm\sthat\sfor\sa\stable\swith\scolumns\sa,\sb,\sc\sand\sd,\s"{a\sb}"\sand\s"-{c\sd}"\sare\shandled\ssimilarly. -D 2020-08-28T11:19:49.438 +C Modify\sthe\sunixShmLock()\sfunction\sto\savoid\siterating\sthrough\sthe\s(possibly\slarge)\sset\sof\sconnections\sto\sthe\ssame\sdatabase\sfile. +D 2020-08-28T19:27:15.533 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -519,7 +519,7 @@ F src/os.c 80e4cf3e5da06be03ca641661e331ce60eeeeabf0d7354dbb1c0e166d0eedbbe F src/os.h 48388821692e87da174ea198bf96b1b2d9d83be5dfc908f673ee21fafbe0d432 F src/os_common.h b2f4707a603e36811d9b1a13278bffd757857b85 F src/os_setup.h 0dbaea40a7d36bf311613d31342e0b99e2536586 -F src/os_unix.c 13553fb5ffbe8c0e60f5d7f553667560b7dece9e31cdfcf8b57b33092a11f226 +F src/os_unix.c 0120726d5ceb10f0a932dbcca3cd2c4c0f110c7f8eab3a788b34cc7accdad6cc F src/os_win.c a2149ff0a85c1c3f9cc102a46c673ce87e992396ba3411bfb53db66813b32f1d F src/os_win.h 7b073010f1451abe501be30d12f6bc599824944a F src/pager.c 3700a1c55427a3d4168ad1f1b8a8b0cb9ace1d107e4506e30a8f1e66d8a1195e @@ -1879,7 +1879,10 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 3f7bbb840de0a9b1ca89288805cb151aea6fcb82efda9ba39f51abf1b17c070b -R 68183e6ade81d2ecd369f241c0d2a0e4 +P 1a04920998368e56276fd0b100be8343609c6ff8a731cf8e26a0490f9c6dabdf +R 2ca214335aa99f522db85aff8974dca1 +T *branch * unixshmlock-opt +T *sym-unixshmlock-opt * +T -sym-trunk * U dan -Z 8ead3559fc701c9e301c92555bb4b25a +Z 2322c300ab82408a3c9b557087654b84 diff --git a/manifest.uuid b/manifest.uuid index 97978cf340..93095b8459 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -1a04920998368e56276fd0b100be8343609c6ff8a731cf8e26a0490f9c6dabdf \ No newline at end of file +e0faddf0dfc3a40b6b94408296dd781dd0264ecc9f2129ce4405438433fb00e0 \ No newline at end of file diff --git a/src/os_unix.c b/src/os_unix.c index c9b59f229a..0eb2d5b000 100644 --- a/src/os_unix.c +++ b/src/os_unix.c @@ -4255,6 +4255,7 @@ struct unixShmNode { 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 */ + int aLock[SQLITE_SHM_NLOCK]; /* # shared locks on slot, -1==excl lock */ #ifdef SQLITE_DEBUG u8 exclMask; /* Mask of exclusive locks held */ u8 sharedMask; /* Mask of shared locks held */ @@ -4795,6 +4796,38 @@ shmpage_out: return rc; } +/* +** Check that the pShmNode->aLock[] array comports with the locking bitmasks +** held by each client. Return true if it does, or false otherwise. This +** is to be used in an assert(). e.g. +** +** assert( assertLockingArrayOk(pShmNode) ); +*/ +#ifdef SQLITE_DEBUG +static int assertLockingArrayOk(unixShmNode *pShmNode){ + unixShm *pX; + int aLock[SQLITE_SHM_NLOCK]; + assert( sqlite3_mutex_held(pShmNode->pShmMutex) ); + + memset(aLock, 0, sizeof(aLock)); + for(pX=pShmNode->pFirst; pX; pX=pX->pNext){ + int i; + for(i=0; iexclMask & (1<sharedMask & (1<=0 ); + aLock[i]++; + } + } + } + + assert( 0==memcmp(pShmNode->aLock, aLock, sizeof(aLock)) ); + return (memcmp(pShmNode->aLock, aLock, sizeof(aLock))==0); +} +#endif + /* ** Change the lock state for a shared-memory segment. ** @@ -4811,10 +4844,10 @@ static int unixShmLock( ){ unixFile *pDbFd = (unixFile*)fd; /* Connection holding shared memory */ unixShm *p = pDbFd->pShm; /* The shared memory being locked */ - unixShm *pX; /* For looping over all siblings */ unixShmNode *pShmNode = p->pShmNode; /* The underlying file iNode */ int rc = SQLITE_OK; /* Result code */ u16 mask; /* Mask of locks to take or release */ + int *aLock = pShmNode->aLock; assert( pShmNode==pDbFd->pInode->pShmNode ); assert( pShmNode->pInode==pDbFd->pInode ); @@ -4853,21 +4886,25 @@ static int unixShmLock( mask = (1<<(ofst+n)) - (1<1 || mask==(1<pShmMutex); + assert( assertLockingArrayOk(pShmNode) ); if( flags & SQLITE_SHM_UNLOCK ){ - u16 allMask = 0; /* Mask of locks held by siblings */ + int ii; + int bUnlock = 1; - /* See if any siblings hold this same lock */ - for(pX=pShmNode->pFirst; pX; pX=pX->pNext){ - if( pX==p ) continue; - assert( (pX->exclMask & (p->exclMask|p->sharedMask))==0 ); - allMask |= pX->sharedMask; + for(ii=ofst; ii((p->sharedMask & (1<sharedMask & (1<1 ); + aLock[ofst]--; } /* Undo the local locks */ @@ -4876,55 +4913,47 @@ static int unixShmLock( p->sharedMask &= ~mask; } }else if( flags & SQLITE_SHM_SHARED ){ - u16 allShared = 0; /* Union of locks held by connections other than "p" */ - - /* Find out which shared locks are already held by sibling connections. - ** If any sibling already holds an exclusive lock, go ahead and return - ** SQLITE_BUSY. - */ - for(pX=pShmNode->pFirst; pX; pX=pX->pNext){ - if( (pX->exclMask & mask)!=0 ){ + assert( n==1 ); + assert( (p->exclMask & (1<sharedMask & mask)==0 ){ + if( aLock[ofst]<0 ){ rc = SQLITE_BUSY; - break; - } - allShared |= pX->sharedMask; - } - - /* Get shared locks at the system level, if necessary */ - if( rc==SQLITE_OK ){ - if( (allShared & mask)==0 ){ + }else if( aLock[ofst]==0 ){ rc = unixShmSystemLock(pDbFd, F_RDLCK, ofst+UNIX_SHM_BASE, n); - }else{ - rc = SQLITE_OK; } - } - /* Get the local shared locks */ - if( rc==SQLITE_OK ){ - p->sharedMask |= mask; + /* Get the local shared locks */ + if( rc==SQLITE_OK ){ + p->sharedMask |= mask; + aLock[ofst]++; + } } }else{ /* Make sure no sibling connections hold locks that will block this - ** lock. If any do, return SQLITE_BUSY right away. - */ - for(pX=pShmNode->pFirst; pX; pX=pX->pNext){ - if( (pX->exclMask & mask)!=0 || (pX->sharedMask & mask)!=0 ){ + ** lock. If any do, return SQLITE_BUSY right away. */ + int ii; + for(ii=ofst; iisharedMask & mask)==0 ); + if( (p->exclMask & (1<sharedMask & mask)==0 ); p->exclMask |= mask; + for(ii=ofst; iipShmMutex); OSTRACE(("SHM-LOCK shmid-%d, pid-%d got %03x,%03x\n", p->id, osGetpid(0), p->sharedMask, p->exclMask)); From 6acdee676551f5fc2fc5ab89bed8c379769eafa9 Mon Sep 17 00:00:00 2001 From: dan Date: Fri, 28 Aug 2020 20:01:06 +0000 Subject: [PATCH 2/2] Fix handling of an xShmLock(SHARED, UNLOCK) call when the caller does not hold any lock on the specified slot, but another connection in the same process holds an EXCLUSIVE. FossilOrigin-Name: 3eb365027b885e1f61965efd53a3643b6ff441ae01e79038a091314516a50dd4 --- manifest | 15 ++++++--------- manifest.uuid | 2 +- src/os_unix.c | 40 +++++++++++++++++++++------------------- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/manifest b/manifest index fd5ff92eb8..5d3cdf5b16 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Modify\sthe\sunixShmLock()\sfunction\sto\savoid\siterating\sthrough\sthe\s(possibly\slarge)\sset\sof\sconnections\sto\sthe\ssame\sdatabase\sfile. -D 2020-08-28T19:27:15.533 +C Fix\shandling\sof\san\sxShmLock(SHARED,\sUNLOCK)\scall\swhen\sthe\scaller\sdoes\snot\shold\sany\slock\son\sthe\sspecified\sslot,\sbut\sanother\sconnection\sin\sthe\ssame\sprocess\sholds\san\sEXCLUSIVE. +D 2020-08-28T20:01:06.517 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -519,7 +519,7 @@ F src/os.c 80e4cf3e5da06be03ca641661e331ce60eeeeabf0d7354dbb1c0e166d0eedbbe F src/os.h 48388821692e87da174ea198bf96b1b2d9d83be5dfc908f673ee21fafbe0d432 F src/os_common.h b2f4707a603e36811d9b1a13278bffd757857b85 F src/os_setup.h 0dbaea40a7d36bf311613d31342e0b99e2536586 -F src/os_unix.c 0120726d5ceb10f0a932dbcca3cd2c4c0f110c7f8eab3a788b34cc7accdad6cc +F src/os_unix.c d707ed2867a2fb32101469327acf3274165d9935e9ab9e27bdab0c1a7d661be7 F src/os_win.c a2149ff0a85c1c3f9cc102a46c673ce87e992396ba3411bfb53db66813b32f1d F src/os_win.h 7b073010f1451abe501be30d12f6bc599824944a F src/pager.c 3700a1c55427a3d4168ad1f1b8a8b0cb9ace1d107e4506e30a8f1e66d8a1195e @@ -1879,10 +1879,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 1a04920998368e56276fd0b100be8343609c6ff8a731cf8e26a0490f9c6dabdf -R 2ca214335aa99f522db85aff8974dca1 -T *branch * unixshmlock-opt -T *sym-unixshmlock-opt * -T -sym-trunk * +P e0faddf0dfc3a40b6b94408296dd781dd0264ecc9f2129ce4405438433fb00e0 +R e6bcf42bbee53ae5585d2ea1ed2212fa U dan -Z 2322c300ab82408a3c9b557087654b84 +Z 13cfa1981b90fb57cb3c9a6bb39ef17d diff --git a/manifest.uuid b/manifest.uuid index 93095b8459..a7a42b45e0 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -e0faddf0dfc3a40b6b94408296dd781dd0264ecc9f2129ce4405438433fb00e0 \ No newline at end of file +3eb365027b885e1f61965efd53a3643b6ff441ae01e79038a091314516a50dd4 \ No newline at end of file diff --git a/src/os_unix.c b/src/os_unix.c index 0eb2d5b000..5419a042a6 100644 --- a/src/os_unix.c +++ b/src/os_unix.c @@ -4888,30 +4888,32 @@ static int unixShmLock( sqlite3_mutex_enter(pShmNode->pShmMutex); assert( assertLockingArrayOk(pShmNode) ); if( flags & SQLITE_SHM_UNLOCK ){ - int ii; - int bUnlock = 1; + if( (p->exclMask|p->sharedMask) & mask ){ + int ii; + int bUnlock = 1; - for(ii=ofst; ii((p->sharedMask & (1<((p->sharedMask & (1<sharedMask & (1<1 ); + aLock[ofst]--; + } + + /* Undo the local locks */ if( rc==SQLITE_OK ){ - memset(&aLock[ofst], 0, sizeof(int)*n); - } - }else if( p->sharedMask & (1<1 ); - aLock[ofst]--; + p->exclMask &= ~mask; + p->sharedMask &= ~mask; + } } - - /* Undo the local locks */ - if( rc==SQLITE_OK ){ - p->exclMask &= ~mask; - p->sharedMask &= ~mask; - } }else if( flags & SQLITE_SHM_SHARED ){ assert( n==1 ); assert( (p->exclMask & (1<