Patchwork localrepo: do not cache auditor/nofsauditor which would make reference cycle

login
register
mail settings
Submitter Yuya Nishihara
Date Aug. 22, 2018, 12:17 p.m.
Message ID <a2388d4e91380a2aec20.1534940237@mimosa>
Download mbox | patch
Permalink /patch/33966/
State Accepted
Headers show

Comments

Yuya Nishihara - Aug. 22, 2018, 12:17 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1534938756 -32400
#      Wed Aug 22 20:52:36 2018 +0900
# Node ID a2388d4e91380a2aec2027be5cda918f1c2a9d9e
# Parent  66f046116105a306f7c701c9a5cf97d7b6c926c0
localrepo: do not cache auditor/nofsauditor which would make reference cycle

Before, self.auditor and self.nofsauditor held self through self._checknested,
and the following code couldn't free a repo by ref-counting.

  def main():
      repo = localrepo.localrepository(uimod.ui(), '../testrepos/hello')
  main()

With this change, the cache of the nofsauditor is limited to a single match
session. I think that's okay as the nofsauditor doesn't do any filesystem
access. Alternatively, maybe we can remove the callback from nofsauditor
since it isn't used unless realfs=True, but I have no idea whether it is
a bug or not.
Augie Fackler - Aug. 28, 2018, 6:18 p.m.
> On Aug 22, 2018, at 8:17 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1534938756 -32400
> #      Wed Aug 22 20:52:36 2018 +0900
> # Node ID a2388d4e91380a2aec2027be5cda918f1c2a9d9e
> # Parent  66f046116105a306f7c701c9a5cf97d7b6c926c0
> localrepo: do not cache auditor/nofsauditor which would make reference cycle

queued, thanks

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -435,14 +435,6 @@  class localrepository(object):
         self.root = self.wvfs.base
         self.path = self.wvfs.join(".hg")
         self.origroot = path
-        # This is only used by context.workingctx.match in order to
-        # detect files in subrepos.
-        self.auditor = pathutil.pathauditor(
-            self.root, callback=self._checknested)
-        # This is only used by context.basectx.match in order to detect
-        # files in subrepos.
-        self.nofsauditor = pathutil.pathauditor(
-            self.root, callback=self._checknested, realfs=False, cached=True)
         self.baseui = baseui
         self.ui = baseui.copy()
         self.ui.copy = baseui.copy # prevent copying repo configuration
@@ -708,6 +700,22 @@  class localrepository(object):
     def _writerequirements(self):
         scmutil.writerequires(self.vfs, self.requirements)
 
+    # Don't cache auditor/nofsauditor, or you'll end up with reference cycle:
+    # self -> auditor -> self._checknested -> self
+
+    @property
+    def auditor(self):
+        # This is only used by context.workingctx.match in order to
+        # detect files in subrepos.
+        return pathutil.pathauditor(self.root, callback=self._checknested)
+
+    @property
+    def nofsauditor(self):
+        # This is only used by context.basectx.match in order to detect
+        # files in subrepos.
+        return pathutil.pathauditor(self.root, callback=self._checknested,
+                                    realfs=False, cached=True)
+
     def _checknested(self, path):
         """Determine if path is a legal nested repository."""
         if not path.startswith(self.root):