Bug#922416: cache.commit() doesn't release the archives lock

[Julian Andres Klode]

On Thu, Mar 07, 2019 at 04:42:04PM +, Marga Manterola wrote:
> Any news here?
> 
> I thought my reproduction case was good enough to show what the problem was.

Sorry, yes. I was at a work sprint on your previous email, and then forgot
about it later! I'll try to solve it this week.

> 
> In the meantime, I've solved my problem by explicitly closing the lock after 
> the call to commit, but I think this shouldn't be 
> needed.

It will probably also fail afterwards :-)

FWIW, one thing about your reproducer is that you should be keeping an
apt_pkg.SystemLock() right from the start until the program ends, so p-a
keeps the frontend lock all the time, preventing race conditions.

(unless you call apt_pkg.init_system() again in which case things
gets screwed up, as it owns the lock, so you'll have to unlock, re-init
and then lock again...).


-- 
debian developer - deb.li/jak | jak-linux.org - free software dev
ubuntu core developer  i speak de, en

Bug#922416: cache.commit() doesn't release the archives lock

[Julian Andres Klode]

On Fri, Feb 15, 2019 at 05:00:58PM +, Marga Manterola wrote:
> Package: python-apt
> Version: 1.8.3
> 
> Hi!
> 
> I have tools that use python-apt to interact with the system. Until
> recently I was using an old version of python-apt (1.4.0~beta3) and it was
> working fine.  I'm now migrating to the latest version (1.8.3) and I've
> encountered a problem with it.
> 
> When testing this version, I found a problem that seems to be related to
> the changes in locking.  The code that's failing is using the cache,
> calling cache.commit and after that's done, calling another command via
> subprocess that also interacts with apt.  The problem that I'm encountering
> is that this other command can't acquire the archives lock.
> 
> Following the code, I've realized that while the commit() function acquires
> a lock [1] (by calling get_lock on the Acquire object), it never actually
> releases this lock.
> 
> From my understanding of the C++ code, the lock is released when the object
> is garbage collected [2] and calling shutdown (which is what the commit()
> code does) is not enough.

This is correct. But the fetcher should be collected at the end of the
SystemLock block, as it goes out of scope and thus ref count drops to 0, so
it's odd that it does not work for you.


> 
> The cache.update() function code is quite different in this regard.  It
> obtains and releases the code explicitly, instead of relying on the Acquire
> object [3]. I don't know if there's a reason behind these two different
> locking behaviors, but it seems to me that the commit one is wrong and it
> would be better to make it acquire and release the lock locally instead of
> relying on the Acquire object being garbage collected

So, this was a bug fix:

https://salsa.debian.org/apt-team/python-apt/commit/618a8e47e6ec74b21ab952da6e85ae327f87cbf7>
 

There might be a solution with explicit locking that also takes care of
keepig the lock open as long as needed. I guess we could do explicit locking
in both fetch_archives and commit().


-- 
debian developer - deb.li/jak | jak-linux.org - free software dev
ubuntu core developer  i speak de, en