[PATCH 1 of 7] lock: add multilock class

[Angel Ezquerra]

On Sun, Nov 29, 2015 at 2:45 PM, Yuya Nishihara <yuya at tcha.org> wrote:
> On Tue, 24 Nov 2015 18:22:25 -0600, Angel Ezquerra 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')
>
> The lock class has no trylock() but _trylock().
>
> And it can just "raise NotImplementedError" if there's no additional
> information.

OK

>> +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 l" will do nothing because "l" still exists in self.locks.

Good catch, and it ties back with get Greg said on another email.

>> +
>> +    def lock(self):
>> +        for l in self.locks:
>> +            l.lock()
>> +
>> +    def trylock(self):
>> +        locked = []
>> +        try:
>> +            for l in self.locks:
>> +                l.trylock()
>> +                locked.append(l)
>> +        except (error.LockHeld, error.LockUnavailable):
>> +            # unlock any locked locks
>> +            for l in locked:
>> +                l.unlock()
>> +            raise
>> +
>> +    def testlock(self):
>> +        """return id of first valid lock, else None"""
>> +        for l in self.locks:
>> +            res = l.testlock()
>> +            if res is not None:
>> +                return res
>> +        return None
>
> It seems multilock.inherit() is missing.
>
>> +    def release(self):
>> +        """release all locks executing their callback functions if any"""
>> +        for l in reversed(self.locks):
>> +            l.release()
>> +        # The postrelease functions typically assume the lock is not held at all
>> +        if not self._parentheld:
>> +            for callback in self.postrelease:
>> +                callback()
>> +
>> +    @property
>> +    def held(self):
>> +        return any(l.held for l in self.locks)
>> +
>> +    @property
>> +    def _parentheld(self):
>> +        return any(l._parentheld for l in self.locks)
>
> I'm not sure if this _parentheld semantics is correct. Should we skip
> postrelease if either one is _parentheld but the other isn't?

I'm really not sure. It'd be great if someone else who knows more
about how locks are used would comment on this.

Cheers,

Angel