[PATCH 1 of 7] lock: add multilock class

[Angel Ezquerra]

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:
>>
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>> # Date 1446166297 -3600
>> #      Fri Oct 30 01:51:37 2015 +0100
>> # Node ID 6b3bf294c85ce407f2bf2f6f18eb07a85aa3ab54
>> # Parent  df9b73d2d444ae82fe8d3fe6cf682a93b2c4a7ef
>> lock: add multilock class
>>
>> This new multilock class is a lock container that behaves as a single
>> lock.
>> This will be useful to implement full shared repositories.
>>
>> The multilock releases its locks in the reverse order that it locks them,
>> which
>> is in the same order that they are passed to the multilock constructor.
>>
>> diff --git a/mercurial/lock.py b/mercurial/lock.py
>> --- a/mercurial/lock.py
>> +++ b/mercurial/lock.py
>> @@ -19,7 +19,22 @@
>>      util,
>>  )
>>
>> -class lock(object):
>> +
>> +class abstractlock(object):
>> +    '''define that interface that all lock classes must follow'''
>> +    def lock(self):
>> +        raise NotImplementedError('abstract method')
>> +
>> +    def trylock(self):
>> +        raise NotImplementedError('abstract method')
>> +
>> +    def testlock(self):
>> +        raise NotImplementedError('abstract method')
>> +
>> +    def release(self):
>> +        raise NotImplementedError('abstract method')
>> +
>> +class lock(abstractlock):
>>      '''An advisory lock held by one process to control access to a set
>>      of files.  Non-cooperating processes or incorrectly written scripts
>>      can ignore Mercurial's locking scheme and stomp all over the
>> @@ -230,6 +245,58 @@
>>                  for callback in self.postrelease:
>>                      callback()
>>
>> +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?

Cheers,

Angel