Patchwork [3,of,3,V4] reposvfs: add a ward to check if locks are properly taken

login
register
mail settings
Submitter Boris Feld
Date July 14, 2017, 2:01 p.m.
Message ID <e132c6e95c50c6f23bd0.1500040910@FB>
Download mbox | patch
Permalink /patch/22332/
State Accepted
Headers show

Comments

Boris Feld - July 14, 2017, 2:01 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1470672882 -7200
#      Mon Aug 08 18:14:42 2016 +0200
# Node ID e132c6e95c50c6f23bd0084e5247032bb8f155ea
# Parent  cea0a88164c6d003b43fe542ec12def2662c1273
# EXP-Topic vfs.ward
reposvfs: add a ward to check if locks are properly taken

we wrap 'repo.svfs.audit' to check for the store lock when accessing file in
'.hg/store' for writing. This caught a couple of instance where the transaction
was released after the lock, we should probably have a dedicated checker for
that case.

Patch

diff -r cea0a88164c6 -r e132c6e95c50 mercurial/localrepo.py
--- a/mercurial/localrepo.py	Tue Jul 11 12:38:17 2017 +0200
+++ b/mercurial/localrepo.py	Mon Aug 08 18:14:42 2016 +0200
@@ -411,6 +411,12 @@ 
         self.svfs = self.store.vfs
         self.sjoin = self.store.join
         self.vfs.createmode = self.store.createmode
+        if (self.ui.configbool('devel', 'all-warnings') or
+            self.ui.configbool('devel', 'check-locks')):
+            if util.safehasattr(self.svfs, 'vfs'): # this is filtervfs
+                self.svfs.vfs.audit = self._getsvfsward(self.svfs.vfs.audit)
+            else: # standard vfs
+                self.svfs.audit = self._getsvfsward(self.svfs.audit)
         self._applyopenerreqs()
         if create:
             self._writerequirements()
@@ -481,6 +487,25 @@ 
             return ret
         return checkvfs
 
+    def _getsvfsward(self, origfunc):
+        """build a ward for self.svfs"""
+        rref = weakref.ref(self)
+        def checksvfs(path, mode=None):
+            ret = origfunc(path, mode=mode)
+            repo = rref()
+            if repo is None or not util.safehasattr(repo, '_lockref'):
+                return
+            if mode in (None, 'r', 'rb'):
+                return
+            if path.startswith(repo.sharedpath):
+                # truncate name relative to the repository (.hg)
+                path = path[len(repo.sharedpath) + 1:]
+            if repo._currentlock(repo._lockref) is None:
+                repo.ui.develwarn('write with no lock: "%s"' % path,
+                                  stacklevel=3)
+            return ret
+        return checksvfs
+
     def close(self):
         self._writecaches()
 
diff -r cea0a88164c6 -r e132c6e95c50 tests/test-devel-warnings.t
--- a/tests/test-devel-warnings.t	Tue Jul 11 12:38:17 2017 +0200
+++ b/tests/test-devel-warnings.t	Mon Aug 08 18:14:42 2016 +0200
@@ -49,6 +49,11 @@ 
   >     with repo.vfs(b'branch', 'a'):
   >         pass
   > 
+  > @command(b'no-lock-write', [], '')
+  > def nolockwrite(ui, repo):
+  >     with repo.svfs(b'fncache', 'a'):
+  >         pass
+  > 
   > @command(b'stripintr', [], '')
   > def stripintr(ui, repo):
   >     lo = repo.lock()
@@ -114,6 +119,9 @@ 
   $ hg no-wlock-write
   devel-warn: write with no wlock: "branch" at: $TESTTMP/buggylocking.py:* (nowlockwrite) (glob)
 
+  $ hg no-lock-write
+  devel-warn: write with no lock: "fncache" at: $TESTTMP/buggylocking.py:* (nolockwrite) (glob)
+
 Stripping from a transaction
 
   $ echo a > a