bookmarks: add appropriate locking (issue1691)
authorMatt Mackall <mpm@selenic.com>
Sat, 20 Jun 2009 16:42:51 -0500
changeset 8862 cd96f159a2d3
parent 8861 90f74b31ed4f
child 8863 7b19c3c0172b
bookmarks: add appropriate locking (issue1691) - make updates of bookmark state locked and atomic - wrap commit so that commit and bookmarks happen under the same lock
hgext/bookmarks.py
--- a/hgext/bookmarks.py	Sat Jun 20 16:42:51 2009 -0500
+++ b/hgext/bookmarks.py	Sat Jun 20 16:42:51 2009 -0500
@@ -64,10 +64,14 @@
         util.copyfile(repo.join('bookmarks'), repo.join('undo.bookmarks'))
     if current(repo) not in refs:
         setcurrent(repo, None)
-    file = repo.opener('bookmarks', 'w+')
-    for refspec, node in refs.iteritems():
-        file.write("%s %s\n" % (hex(node), refspec))
-    file.close()
+    wlock = repo.wlock()
+    try:
+        file = repo.opener('bookmarks', 'w', atomictemp=True)
+        for refspec, node in refs.iteritems():
+            file.write("%s %s\n" % (hex(node), refspec))
+        file.rename()
+    finally:
+        wlock.release()
 
 def current(repo):
     '''Get the current bookmark
@@ -106,9 +110,13 @@
         return
     if mark not in refs:
         mark = ''
-    file = repo.opener('bookmarks.current', 'w+')
-    file.write(mark)
-    file.close()
+    wlock = repo.wlock()
+    try:
+        file = repo.opener('bookmarks.current', 'w', atomictemp=True)
+        file.write(mark)
+        file.rename()
+    finally:
+        wlock.release()
     repo._bookmarkcurrent = mark
 
 def bookmark(ui, repo, mark=None, rev=None, force=False, delete=False, rename=None):
@@ -242,26 +250,30 @@
         def commit(self, *k, **kw):
             """Add a revision to the repository and
             move the bookmark"""
-            node  = super(bookmark_repo, self).commit(*k, **kw)
-            if node is None:
-                return None
-            parents = repo.changelog.parents(node)
-            if parents[1] == nullid:
-                parents = (parents[0],)
-            marks = parse(repo)
-            update = False
-            for mark, n in marks.items():
-                if ui.configbool('bookmarks', 'track.current'):
-                    if mark == current(repo) and n in parents:
-                        marks[mark] = node
-                        update = True
-                else:
-                    if n in parents:
-                        marks[mark] = node
-                        update = True
-            if update:
-                write(repo, marks)
-            return node
+            wlock = self.wlock() # do both commit and bookmark with lock held
+            try:
+                node  = super(bookmark_repo, self).commit(*k, **kw)
+                if node is None:
+                    return None
+                parents = repo.changelog.parents(node)
+                if parents[1] == nullid:
+                    parents = (parents[0],)
+                marks = parse(repo)
+                update = False
+                for mark, n in marks.items():
+                    if ui.configbool('bookmarks', 'track.current'):
+                        if mark == current(repo) and n in parents:
+                            marks[mark] = node
+                            update = True
+                    else:
+                        if n in parents:
+                            marks[mark] = node
+                            update = True
+                if update:
+                    write(repo, marks)
+                return node
+            finally:
+                wlock.release()
 
         def addchangegroup(self, source, srctype, url, emptyok=False):
             parents = repo.dirstate.parents()