Re: Transaction Set clean does not actually clean

On Dec 7, 2007, at 4:45 PM, Mitko Haralanov wrote:

> On Fri, 7 Dec 2007 15:12:31 -0500
> Jeff Johnson <[email protected]> wrote:
>
>> Calling closeDB() (or doing any operation on a rpmdb) explicitly  
>> is not
>> recommended. Delete the transaction set and create a new transaction
>> instead. The reason is that there are global aspects to concurrent
>> access of an rpmdb that few application programs are prepared to
>> implement properly.
>
> I tried that, however, this creates problems with multi-threaded
> applications. If one of the threads deletes the TransactionSet
> instance, then the other threads throw exceptions.
>

OK.

> Lock and unlocking the TransactionSet instance doesn't really help
> since holding a lock does not guarantee that the instance will exist.
>

You need an additional reference, not a lock, to keep yer
threads "happy". rpmlib itself is not thread safe, never has been,
don't fool yourself. You can likely still do multithreading through
rpmlib if you design in a Big Lock on using rpmlib, or are otherwise
careful abt locking on python, rather than rpmlib, objects.

Most applications (yours included afaict) cannot lock an rpmdb
effectively. For starters, with INIT_CDB, the locks are on the cursor,
not "The Database". You're not even close to being able to set
cursor locks.

OTOH, one can rather easily arrange for a global exclusive/shared
fcntl lock that (if done correctly) the rpm CLI and other applications
that use an rpmdb through rpmlib will honor.

> What I did try is to create a wrapper class around the TransactionSet
> instance (pasted below) that will try to do the right thing.
> Unfortunately, this does not work either in cases where the
> TransactionSet operation returns a match object (for example), since
> working with the match object does depend on the TransactionSet
> instance to still be around.
>

Yes an rpmdb iterator match will likely become very unhappy if you
manually open/close an rpmdb out-of-band.

> class Transaction:
>     def __init__ (self, db=None):
>         self.ts = None
>         self.ts_lock = Lock ()
>         self.db = db
>         self.func = None
>
>     def __call__ (self, *args):
>         return False
>
>     # This method will acquire the lock, record the TransactionSet
>     # method being called by the caller and return the pointer to
>     # the call_ts_func () function, which will do all the work.
>     def __getattr__ (self, name):
>         self.ts_lock.acquire ()
>         self.func = name
>         return self.call_ts_func
>
>     # This method serves as a wrapper around the actual TransactionSet
>     # method being called. It will open the database, call the
>     # TransactionSet method, close the databse, and then release the
>     # lock acquired in the __getattr__ () function.
>     def call_ts_func (self, *args):
>         if not self.ts:
>             if self.db: rpm.addMacro ("_dbpath", self.db)
>             self.ts = rpm.TransactionSet ()
>             self.ts.openDB ()

Lose this call. see below.

>             if self.db: rpm.delMacro ("_dbpath")
>

Likely unneeded unless you are doing something with
a rpmdb on a private path. Set once and ignore if possible.

>         func = self.ts.__getattribute__ (self.func)
>         rs = func (*args)
>
>         self.func = None
>         self.ts.clean ()


OK, so you want ts.clean() to "work".

Translating python -> C, the binding is just a wrapper onto
     rpmtsClean().

Why do you think rpmtsClean is not "cleaning", as in removing
all the references from the transaction set to the transaction elements
(aka packages) in the transaction set?

Do you have other references to rpmte objects? rpmlib data
structures are ref-counted, so you also need to delete the python
references to truly get rid of (as in free'ing) the underlying data  
structure.


>         self.ts.closeDB ()

Lose this call. closeDB() has a side effect, it prevents rpmlib from  
doing
the lazy opens necessary for "normal"  rpmlib functioning. rpmlib will
uniqify references and share the underlying rpmdb object as
long as you don't go mucking about with closeDB().

OTOH, muck about as you wish, just don't expect too much from
the python bindings if you insist on using openDB() and closeDB().

>         del self.ts
>         self.ts = None
>         self.ts_lock.release ()

As noted, you seem to need an additional persistent reference on a  
transaction set, not a lock,
to keep yr threads happy.

>         return rs
>

hth

73 de Jeff