On Dec 4, 2006, at 1:56 PM, Tony Nelson wrote:
At 12:37 AM -0500 12/4/06, Jeff Johnson wrote:
On Dec 3, 2006, at 11:12 PM, Tony Nelson wrote:
While I think the code is insanely clever, and I'm quite pleased
that rpm is surviving as well as it is in spite of unanticipated
and bizzarre uses of the implementation, the code is solving
entirely the wrong problem, and triggering a lot of instability.
I don't quite see that this code is a problem. It only runs at
task time
(not at signal time), when the object is being reclaimed by the
Python
interpreter. It should be like any other call to close the
database. It
can't happen inside a call to rpmlib (unless there is more than one
thread). Am I misunderstanding the issue?
The intent of
ts.CloseDB() # <-- this is unnecessary btw
del ts
is to keep rpm from "stealing" signal handlers, on last
rpmdb close the original signal handlers are lazily restored.
I have heard no other reason for the "del ts" change, transaction.py
used to have a persistent transaction which kept an rpmdb
open as long as needed.
You may well be right about how it's being used, but the
TransactionWrapper
class /says/ its purpose is to allow easy instrumentation of
rpm.Transaction. It appears to be the "owner" of one of them,
which it
takes pains to close when it goes away. (If it really "owns" a
TransactionSet when it returns it to use as an iterator.) (There's
also a
global ts which isn't being used for anything?) If it is being
(mis)used
to do what you say, that would be by the code that holds a
reference to it
choosing to forget that reference.
Or are you saying that "del ts" would not do the ts.CloseDB()? I'm
assuming that it would close it, which makes the destructor
unnecessary.
The ts.CloseDB() is unnecessary, "del ts" will accomplish the same call
to rpmdbClose() when a transaction is freed.
Way down underneath the hood, rpmlib is maintaining a link list of
open rpmdb databases, and associating already open rpmdb's with
new transactions, maintaining a refcount on used rpmdb objects, so that
users of rpm-python can freely create/destroy transactions without
having to worry about the underlying attached rpmdb database
environment.
The code snippet forces the ts <-> rpmdb relation to be one-to-one,
which is okay. I worry about the use of ts.CloseDB(), which has
other side-effects, namely any database that is manually closed
using ts.CloseDB() will never be lazily open'ed again.
With all the other mess of maintaining link lists and refcounts and
associating already existing rpmdb's with newly created ts objects,
I'd rather *NOT* also have to worry about about raciness on a
manually closed rpmdb that prevents lazy reopens.
AFAIK (and I have looked), the
ts.CloseDB()
is unnecessary to yum, the same action will be performed by
del ts
Meanwhile, the eventual bestest fix is going to be to handle
signals through other means and making the ts object more
persistent (thereby preserving the database environment's
concurrency guarantee of "multiple readers or single writer",
and being higher performing as well.
Got that?
73 de Jeff
_______________________________________________
Rpm-list mailing list
Rpm-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/rpm-list
