Re: [gentoo-portage-dev] [PATCH] bindbapi: add reentrant lock method (bug 685236)

[Zac Medico]

On 5/7/19 7:55 AM, Alec Warner wrote:
>
>
> On Mon, May 6, 2019 at 8:15 PM Zac Medico <zmedico@g.o
> <mailto:zmedico@g.o>> wrote:
>
>     Add a reentrant lock/unlock method which is useful for things like
>     emaint binhost and eclean-pkg. The vardbapi class already provides
>     lock/unlock methods that behave the same way.
>
>
> Curious why we are not doing more encapsulation here.
>
> I'd suggest a new class type in locks.py:
>
> class RefCountedLock(object): ...
> # This implements the Lock() / Unlock() interface, as well as perhaps
> __enter__ and __exit__.
> # It uses a lockfile at path and a reference count.
>
> Then in bintree's init, set self.lock to
> locks.refCountedLock(self.bintree._pkgindex_file)
>
> def lock(self): # on bintree
>   self._lock.Lock()
>
> def unlock(self):
>   self._lock.Unlock()

Yes, splitting out a reference counted lock class like this is a good
idea. I'll do that.

> Also curious why we are not implementing enter and exit so we can avoid
> unbalanced pairs by using context managers.
>
> e.g. in match(), we could likely write:
>
> with self.dbapi.lock():
>   # try to match
>   update_pkgindex = self._populate_local()
>   if update_pkgindex:
>     self._pkgindex_write(update_pkgindex)
>
> If the block raises an exception, __exit__ is still called, so we can
> eliminate the finally: blocks and replace them with a careful __exit__
> implementation?
>
> -A

Yeah, the reference counted lock class could have a method that returns
a context manager which increments/decrements the lock count. The
context manager can be implemented with the contextlib.contextmanager
decorator like this:

@contextlib.contextmanager
def contextmanager(self):
    self.lock()
    try:
        yield self
    finally:
        self.unlock()

-- 
Thanks,
Zac