Patchwork [2,of,2] localrepo: don't reference transaction from hook closure (issue5043)

login
register
mail settings
Submitter Gregory Szorc
Date Jan. 17, 2016, 8:47 p.m.
Message ID <91ce891fcdcde6916639.1453063654@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/12818/
State Superseded
Headers show

Comments

Gregory Szorc - Jan. 17, 2016, 8:47 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1453063512 28800
#      Sun Jan 17 12:45:12 2016 -0800
# Node ID 91ce891fcdcde6916639914d865101ececa8855f
# Parent  0723bc793d3bd2d4b52a748d68561e8ea0c99e05
localrepo: don't reference transaction from hook closure (issue5043)

Before, the hook() closure (which is called as part of locking hooks)
would maintain a reference to a transaction instance (which should be
finalized by the time lock hooks are called). This would create
problems from the garbage collector and leak transaction instances
in certain usage scenarios, including in `hg convert`.

Creating a reference to the hook arguments dict prevents the closure
over the transaction instance and prevents the leak of this object.

This and the previous patch fix the majority of leaks in `hg convert`.
However, we still appear to be leaking the hook() closure function
as well as some other small objects. So the issue should remain open.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1080,19 +1080,22 @@  class localrepository(object):
         tr.hookargs['txnid'] = txnid
         # note: writing the fncache only during finalize mean that the file is
         # outdated when running hooks. As fncache is used for streaming clone,
         # this is not expected to break anything that happen during the hooks.
         tr.addfinalize('flush-fncache', self.store.write)
         def txnclosehook(tr2):
             """To be run if transaction is successful, will schedule a hook run
             """
+            # Don't reference tr2 in hook() or we'll create a cycle.
+            hookargs = tr2.hookargs
+
             def hook():
                 reporef().hook('txnclose', throw=False, txnname=desc,
-                               **tr2.hookargs)
+                               **hookargs)
             reporef()._afterlock(hook)
         tr.addfinalize('txnclose-hook', txnclosehook)
         def txnaborthook(tr2):
             """To be run if transaction is aborted
             """
             reporef().hook('txnabort', throw=False, txnname=desc,
                            **tr2.hookargs)
         tr.addabort('txnabort-hook', txnaborthook)