avoid stageJournal escaping withOtherTmp

This is only done for correctness sake; I don't see any way that it
would have caused a problem here. The jlog file escaped withOtherTmp
so another process could swoop in and delete it, but the file is only
used as a buffer for a list of filenames, and its handle gets rewound
and they're read back out, which will still work even if it's already
been deleted.

The only reason I didn't just pre-delete the file and keep the handle
open is I'm not sure that works on all OS's (eg Windows). If there was
a problem that this fixed it might involve an OS that doesn't support
deleting an open file or something like that.
This commit is contained in:
Joey Hess 2019-05-07 11:57:12 -04:00
parent ce83783fcc
commit 2a41712ef1
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
2 changed files with 18 additions and 18 deletions

View file

@ -177,9 +177,8 @@ updateTo' pairs = do
where where
excludeset s = filter (\(r, _) -> S.notMember r s) excludeset s = filter (\(r, _) -> S.notMember r s)
isnewer (r, _) = inRepo $ Git.Branch.changed fullname r isnewer (r, _) = inRepo $ Git.Branch.changed fullname r
go branchref dirty tomerge jl = withIndex $ do go branchref dirty tomerge jl = stagejournalwhen dirty jl $ do
let (refs, branches) = unzip tomerge let (refs, branches) = unzip tomerge
cleanjournal <- if dirty then stageJournal jl else return noop
merge_desc <- if null tomerge merge_desc <- if null tomerge
then commitMessage then commitMessage
else return $ "merging " ++ else return $ "merging " ++
@ -203,7 +202,9 @@ updateTo' pairs = do
else commitIndex jl branchref merge_desc commitrefs else commitIndex jl branchref merge_desc commitrefs
) )
addMergedRefs tomerge addMergedRefs tomerge
liftIO cleanjournal stagejournalwhen dirty jl a
| dirty = stageJournal jl a
| otherwise = withIndex a
{- Gets the content of a file, which may be in the journal, or in the index {- Gets the content of a file, which may be in the journal, or in the index
- (and committed to the branch). - (and committed to the branch).
@ -281,11 +282,10 @@ commit = whenM journalDirty . forceCommit
{- Commits the current index to the branch even without any journalled {- Commits the current index to the branch even without any journalled
- changes. -} - changes. -}
forceCommit :: String -> Annex () forceCommit :: String -> Annex ()
forceCommit message = lockJournal $ \jl -> do forceCommit message = lockJournal $ \jl ->
cleanjournal <- stageJournal jl stageJournal jl $ do
ref <- getBranch ref <- getBranch
withIndex $ commitIndex jl ref message [fullname] commitIndex jl ref message [fullname]
liftIO cleanjournal
{- Commits the staged changes in the index to the branch. {- Commits the staged changes in the index to the branch.
- -
@ -447,9 +447,9 @@ setIndexSha ref = do
writeLogFile f $ fromRef ref ++ "\n" writeLogFile f $ fromRef ref ++ "\n"
runAnnexHook postUpdateAnnexHook runAnnexHook postUpdateAnnexHook
{- Stages the journal into the index and returns an action that will {- Stages the journal into the index, and runs an action that
- clean up the staged journal files, which should only be run once - commits the index to the branch. Note that the action is run
- the index has been committed to the branch. - inside withIndex so will automatically use the branch's index.
- -
- Before staging, this removes any existing git index file lock. - Before staging, this removes any existing git index file lock.
- This is safe to do because stageJournal is the only thing that - This is safe to do because stageJournal is the only thing that
@ -458,17 +458,18 @@ setIndexSha ref = do
- stale, and the journal must contain any data that was in the process - stale, and the journal must contain any data that was in the process
- of being written to the index file when it crashed. - of being written to the index file when it crashed.
-} -}
stageJournal :: JournalLocked -> Annex (IO ()) stageJournal :: JournalLocked -> Annex () -> Annex ()
stageJournal jl = withIndex $ do stageJournal jl commitindex = withIndex $ withOtherTmp $ \tmpdir -> do
prepareModifyIndex jl prepareModifyIndex jl
g <- gitRepo g <- gitRepo
let dir = gitAnnexJournalDir g let dir = gitAnnexJournalDir g
(jlogf, jlogh) <- openjlog (jlogf, jlogh) <- openjlog tmpdir
h <- hashObjectHandle h <- hashObjectHandle
withJournalHandle $ \jh -> withJournalHandle $ \jh ->
Git.UpdateIndex.streamUpdateIndex g Git.UpdateIndex.streamUpdateIndex g
[genstream dir h jh jlogh] [genstream dir h jh jlogh]
return $ cleanup dir jlogh jlogf commitindex
liftIO $ cleanup dir jlogh jlogf
where where
genstream dir h jh jlogh streamer = readDirectory jh >>= \case genstream dir h jh jlogh streamer = readDirectory jh >>= \case
Nothing -> return () Nothing -> return ()
@ -490,8 +491,7 @@ stageJournal jl = withIndex $ do
mapM_ (removeFile . (dir </>)) stagedfs mapM_ (removeFile . (dir </>)) stagedfs
hClose jlogh hClose jlogh
nukeFile jlogf nukeFile jlogf
openjlog = withOtherTmp $ \tmpdir -> openjlog tmpdir = liftIO $ openTempFile tmpdir "jlog"
liftIO $ openTempFile tmpdir "jlog"
{- This is run after the refs have been merged into the index, {- This is run after the refs have been merged into the index,
- but before the result is committed to the branch. - but before the result is committed to the branch.

View file

@ -18,5 +18,5 @@ This is a fairly new problem because the code to have other processes
cleanup stale othertmp files was only added a couple months back. cleanup stale othertmp files was only added a couple months back.
I audited other uses of withOtherTmp, and the only other problem I found is I audited other uses of withOtherTmp, and the only other problem I found is
similar, in Annex.Branch.stageJournal. similar, in Annex.Branch.stageJournal. Fixed that one.
--[[Joey]] --[[Joey]]