Patchwork [3,of,3,STABLE] devel-warn: issue a warning when writing bookmarks without holding the wlock

login
register
mail settings
Submitter Pierre-Yves David
Date Aug. 12, 2015, 12:06 a.m.
Message ID <bb267085bdd69fff0f5f.1439338005@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/10193/
State Accepted
Headers show

Comments

Pierre-Yves David - Aug. 12, 2015, 12:06 a.m.
# HG changeset patch
# User Laurent Charignon <lcharignon@fb.com>
# Date 1438433019 25200
#      Sat Aug 01 05:43:39 2015 -0700
# Branch stable
# Node ID bb267085bdd69fff0f5fa4091bbc223889d57c0b
# Parent  3cfb0c92590589c9ccdef1884b6133d0d6303805
devel-warn: issue a warning when writing bookmarks without holding the wlock

I saw an issue in an extension that we develop where we were writing bookmarks
without holding the wlock. Another extension was taking a lock at the same time
and wiped out the bookmarks we were about to write. This patch adds a
devel-warning to urge people to fix their invalide code.
Matt Mackall - Aug. 13, 2015, 9:34 p.m.
On Tue, 2015-08-11 at 17:06 -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@fb.com>
> # Date 1438433019 25200
> #      Sat Aug 01 05:43:39 2015 -0700
> # Branch stable
> # Node ID bb267085bdd69fff0f5fa4091bbc223889d57c0b
> # Parent  3cfb0c92590589c9ccdef1884b6133d0d6303805
> devel-warn: issue a warning when writing bookmarks without holding the wlock
> 
> I saw an issue in an extension that we develop where we were writing bookmarks
> without holding the wlock. Another extension was taking a lock at the same time
> and wiped out the bookmarks we were about to write. This patch adds a
> devel-warning to urge people to fix their invalide code.

I've queued the first two for default. The last obviously doesn't belong
on stable, so I'll try to juggle it into the right place after I merge.

Patch

diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -77,10 +77,15 @@  class bmstore(dict):
 
         We also store a backup of the previous state in undo.bookmarks that
         can be copied back on rollback.
         '''
         repo = self._repo
+        if (repo.ui.configbool('devel', 'all-warnings')
+                or repo.ui.configbool('devel', 'check-locks')):
+            l = repo._wlockref and repo._wlockref()
+            if l is None or not l.held:
+                repo.ui.develwarn('bookmarks write with no wlock')
         self._writerepo(repo)
         repo.invalidatevolatilesets()
 
     def _writerepo(self, repo):
         """Factored out for extensibility"""