[PATCH 02 of 11] commitctx: stop using weakref proxy for transaction

[Pierre-Yves David]

On 7/27/20 12:18 AM, Gregory Szorc wrote:
> On Fri, Jul 24, 2020 at 8:49 AM Pierre-Yves David 
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>> 
> wrote:
> 
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david at octobus.net
>     <mailto:pierre-yves.david at octobus.net>>
>     # Date 1595587952 -7200
>     #      Fri Jul 24 12:52:32 2020 +0200
>     # Node ID 76a585b26acdaf884e1c40252e351b1d45cbbcf1
>     # Parent  2727e91ffa6e9063bd9c29671b5008cfef22dd97
>     # EXP-Topic commitctx-cleanup-2
>     # Available At https://foss.heptapod.net/octobus/mercurial-devel/
>     #              hg pull
>     https://foss.heptapod.net/octobus/mercurial-devel/ -r 76a585b26acd
>     commitctx: stop using weakref proxy for transaction
> 
> 
> I have concerns about this patch.
> 
> I believe the reason we continue to use a weak ref here is due to cycles 
> leading to leaked objects due to failure to garbage collect.

We need weakref usage at higher level to avoid cycle. However we are 
quite low level here and I could not found any justification for the 
weakref other than the one listed in the commit I pointed.

Do you have some concrete piece of code under this one that concerns you 
especially?

> I believe there are still places where we leak transaction objects 
> because of cycles. Try doing a `hg convert` between 2 Mercurial repos 
> involving thousands of changesets. I suspect this change may make the 
> memory leak worse since the garbage collector isn't able to break cycles 
> as easily.

I agree we still have memory leak in some place, however I don't think 
they would originate from this code.

-- 
Pierre-Yves David