Patchwork D10504: dirstateguard: use mktemp-like functionality to generate the backup filenames

login
register
mail settings
Submitter phabricator
Date April 20, 2021, 8:08 p.m.
Message ID <differential-rev-PHID-DREV-lw35lx75rsn6jbpjqsni-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48821/
State Superseded
Headers show

Comments

phabricator - April 20, 2021, 8:08 p.m.
spectral created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Previously these were generated with names like:
  `dirstate.backup.commit.<memory address of dirstateguard>`
  
  This could cause problems if two hg commands ran at the same time that used the
  same memory address, (which is apparently not uncommon if chg is involved), as
  memory addresses are not unique across processes.
  
  This issue was reported in the post-review comments on
  http://phab.mercurial-scm.org/D9952.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D10504

AFFECTED FILES
  mercurial/dirstateguard.py

CHANGE DETAILS




To: spectral, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/mercurial/dirstateguard.py b/mercurial/dirstateguard.py
--- a/mercurial/dirstateguard.py
+++ b/mercurial/dirstateguard.py
@@ -7,6 +7,7 @@ 
 
 from __future__ import absolute_import
 
+import os
 from .i18n import _
 
 from . import (
@@ -34,11 +35,12 @@ 
         self._repo = repo
         self._active = False
         self._closed = False
-        self._backupname = b'dirstate.backup.%s.%d' % (name, id(self))
-        self._narrowspecbackupname = b'narrowspec.backup.%s.%d' % (
-            name,
-            id(self),
-        )
+        def getname(prefix):
+            fd, fname = repo.vfs.mkstemp(prefix=prefix)
+            os.close(fd)
+            return fname
+        self._backupname = getname(b'dirstate.backup.%s.' % name)
+        self._narrowspecbackupname = getname(b'narrowspec.backup.%s.' % name)
         repo.dirstate.savebackup(repo.currenttransaction(), self._backupname)
         narrowspec.savewcbackup(repo, self._narrowspecbackupname)
         self._active = True