[Bug 5045] New: txnclose hooks accumulate, can fire multiple times per lock

mercurial-bugs at selenic.com mercurial-bugs at selenic.com
Sun Jan 17 16:09:56 CST 2016


            Bug ID: 5045
           Summary: txnclose hooks accumulate, can fire multiple times per
           Product: Mercurial
           Version: default branch
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: bug
          Priority: normal
         Component: Mercurial
          Assignee: bugzilla at selenic.com
          Reporter: gregory.szorc at gmail.com
                CC: mercurial-devel at selenic.com

As part of investigating memory leaks in bug 5043, I found some wonky behavior
in transaction-lock hook interaction.

In that bug, I was tracking down a leak of the hook() closure from

>From localrepository.transaction():

        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,
        tr.addfinalize('txnclose-hook', txnclosehook)

So, when the transaction finalizes, we register a post-finalize hook in the
lock object. This hook calls the "txnclose" hook.

There are a few issues here.

First, why are we running the "txnclose" hook during lock release? Shouldn't it
fire during, well, transaction release?

Second, for `hg convert`'s use case of multiple transactions per lock, we're
registering 1 post lock release hook for each transaction opened. When the lock
finally releases, we could be running thousands of "txnclose" hooks! So, we're
not really leaking the hook() closure per se - we're just accumulating them
because they never run!

This behavior feels wonky to me. But it could be a feature, not a bug. It might
be worth documenting if this is the intended behavior.

You are receiving this mail because:
You are on the CC list for the bug.

More information about the Mercurial-devel mailing list