[PATCH 1 of 7] lock: add multilock class

Yuya Nishihara yuya at tcha.org
Sun Dec 13 02:35:59 CST 2015


On Sat, 12 Dec 2015 16:52:13 -0500, Gregory Szorc wrote:
> > On Dec 7, 2015, at 06:10, Angel Ezquerra <angel.ezquerra at gmail.com> wrote:
> >> On Sat, Nov 28, 2015 at 10:57 PM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
> >> On Tue, Nov 24, 2015 at 4:22 PM, Angel Ezquerra <angel.ezquerra at gmail.com>
> >> wrote:
> >>> +class multilock(abstractlock):
> >>> +    """a group of locks that behave as one"""
> >>> +    def __init__(self, *locks):
> >>> +        self.locks = locks
> >>> +        self.postrelease = []
> >>> +
> >>> +    def __del__(self):
> >>> +        for l in reversed(self.locks):
> >>> +            del l
> >>> +        self.locks = []
> >> 
> >> 
> >> __del__ is kind of evil since it can block garbage collection from
> >> occurring, causing memory leaks. See
> >> https://docs.python.org/2/library/gc.html#gc.garbage.
> >> 
> >> If you need to implement __del__ here to conform to the existing lock API,
> >> so be it. But I would strongly prefer we refactor the lock API to use
> >> context managers (preferred) or a cleanup function that isn't named __del__.
> > 
> > Greg, thanks for the review. Sorry it took me a while to get back to you.
> > 
> > I agree with the sentiment, but I think that refactoring the lock API
> > is well beyond the scope of what I am trying to do. I think is should
> > be handled separately, on another future set of patches. Do you think
> > that makes sense?
> 
> I agree it is scope bloat. But I defer to someone with queueing privileges
> for the final word.

Because I've not joined the discussion about the subrepo cache, I'm not sure
this multilock class is the right path. It won't be easy to write multilock
fully conforming to the lock interface.

Anyway this __del__ has no effect, it can simply be removed.


More information about the Mercurial-devel mailing list