Revert "fix too early close of shared lock file"

This reverts commit 66b2536ea0.

I misunderstood commit ac56a5c2a0
and caused a FD leak when pid locking is not used.

A LockHandle contains an action that will close the underlying lock
file, and that action is run when it is closed. In the case of a shared
lock, the lock file is opened once for each LockHandle, and only
the one for the LockHandle that is being closed will be closed.
This commit is contained in:
Joey Hess 2021-12-06 12:48:37 -04:00
parent 00625caf87
commit ae4c56b28a
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
3 changed files with 21 additions and 9 deletions

View file

@ -6,7 +6,6 @@ git-annex (8.20211124) UNRELEASED; urgency=medium
erroring out. erroring out.
* export: Avoid unncessarily re-exporting non-annexed files that were * export: Avoid unncessarily re-exporting non-annexed files that were
already exported. already exported.
* Fix locking bug introduced in version 8.20200814.
* Fix locking problems when annex.pidlock is set and concurrency is * Fix locking problems when annex.pidlock is set and concurrency is
enabled eg with -J. enabled eg with -J.

View file

@ -28,7 +28,6 @@ import System.FilePath.ByteString (RawFilePath)
import qualified Data.Map.Strict as M import qualified Data.Map.Strict as M
import Control.Concurrent.STM import Control.Concurrent.STM
import Control.Exception import Control.Exception
import Control.Monad
type LockFile = RawFilePath type LockFile = RawFilePath
@ -129,8 +128,8 @@ getLockStatus pool file getdefault checker = do
Nothing -> getdefault Nothing -> getdefault
Just restore -> bracket_ (return ()) restore checker Just restore -> bracket_ (return ()) restore checker
-- Only runs action to close underlying lock file when this is the last -- Releases the lock. When it is a shared lock, it may remain locked by
-- user of the lock, and when the lock has not already been closed. -- other LockHandles.
-- --
-- Note that the lock pool is left empty while the CloseLockFile action -- Note that the lock pool is left empty while the CloseLockFile action
-- is run, to avoid race with another thread trying to open the same lock -- is run, to avoid race with another thread trying to open the same lock
@ -139,15 +138,15 @@ releaseLock :: LockHandle -> IO ()
releaseLock h = go =<< atomically (tryTakeTMVar h) releaseLock h = go =<< atomically (tryTakeTMVar h)
where where
go (Just (pool, file, closelockfile)) = do go (Just (pool, file, closelockfile)) = do
(m, lastuser) <- atomically $ do m <- atomically $ do
m <- takeTMVar pool m <- takeTMVar pool
return $ case M.lookup file m of return $ case M.lookup file m of
Just (LockStatus mode n firstlocksem) Just (LockStatus mode n firstlocksem)
| n == 1 -> (M.delete file m, True) | n == 1 -> (M.delete file m)
| otherwise -> | otherwise ->
(M.insert file (LockStatus mode (pred n) firstlocksem) m, False) (M.insert file (LockStatus mode (pred n) firstlocksem) m)
Nothing -> (m, True) Nothing -> m
() <- when lastuser closelockfile () <- closelockfile
atomically $ putTMVar pool m atomically $ putTMVar pool m
-- The LockHandle was already closed. -- The LockHandle was already closed.
go Nothing = return () go Nothing = return ()

View file

@ -0,0 +1,14 @@
[[!comment format=mdwn
username="joey"
subject="""comment 9"""
date="2021-12-06T16:36:45Z"
content="""
Unfortunately, [[!commit 66b2536ea0aa5c88f5b744eeace3322a8a4a10b6]]
broke shared locking when pid locks are not used, causing a FD leak,
and had to be reverted. Which further breaks concurrent pid locking.
At this point, when -J is used with pidlock, it may drop the lock but
still think it has it held internally. Comment #7 found another way that
could happen, and the conclusion is that the pid lock must migrate away
from using the lock pool.
"""]]