Fix loss of valid dbus objects after unregisterObject

Partial revert of 3c6bb0ed8b and
57aed703d2.

When registering an object that was previously unregistered but not yet
garbage collected, the activeChildren count on the parent node was not
incremented, which could result in other registered objects disappearing
after a later unregisterObject.

Copying objects in the tree is not free, but it's not expensive enough
or used frequently enough to justify that error-prone logic. It's much
safer to simply remove objects immediately.

Change-Id: I3dc59c2ebd07b237518424fcd8ea7371a22d6d15
Reviewed-by: Robin Burchell <robin+qt@viroteck.net>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
bb10
John Brooks 2013-09-04 00:21:32 -06:00 committed by The Qt Project
parent 2171b057e3
commit 6827728d5a
3 changed files with 11 additions and 40 deletions

View File

@ -804,7 +804,7 @@ bool QDBusConnection::registerObject(const QString &path, QObject *object, Regis
return false;
if (options & QDBusConnectionPrivate::VirtualObject) {
if (options & SubPath && node->activeChildren)
if (options & SubPath && !node->children.isEmpty())
return false;
} else {
if ((options & ExportChildObjects && !node->children.isEmpty()))
@ -842,7 +842,6 @@ bool QDBusConnection::registerObject(const QString &path, QObject *object, Regis
}
} else {
// add entry
++node->activeChildren;
node = node->children.insert(it, pathComponents.at(i));
}

View File

@ -140,16 +140,16 @@ public:
{
typedef QVector<ObjectTreeNode> DataList;
inline ObjectTreeNode() : obj(0), flags(0), activeChildren(0) { }
inline ObjectTreeNode() : obj(0), flags(0) { }
inline ObjectTreeNode(const QString &n) // intentionally implicit
: name(n), obj(0), flags(0), activeChildren(0) { }
: name(n), obj(0), flags(0) { }
inline ~ObjectTreeNode() { }
inline bool operator<(const QString &other) const
{ return name < other; }
inline bool operator<(const QStringRef &other) const
{ return QStringRef(&name) < other; }
inline bool isActive() const
{ return obj || activeChildren; }
{ return obj || !children.isEmpty(); }
QString name;
union {
@ -157,7 +157,6 @@ public:
QDBusVirtualObject *treeNode;
};
int flags;
int activeChildren;
DataList children;
};

View File

@ -587,46 +587,22 @@ bool QDBusConnectionPrivate::handleMessage(const QDBusMessage &amsg)
return false;
}
static void garbageCollectChildren(QDBusConnectionPrivate::ObjectTreeNode &node)
{
int size = node.children.count();
if (node.activeChildren == 0) {
// easy case
node.children.clear();
} else if (size > node.activeChildren * 3 || (size > 20 && size * 2 > node.activeChildren * 3)) {
// rewrite the vector, keeping only the active children
// if the vector is large (> 20 items) and has one third of inactives
// or if the vector is small and has two thirds of inactives.
QDBusConnectionPrivate::ObjectTreeNode::DataList::Iterator end = node.children.end();
QDBusConnectionPrivate::ObjectTreeNode::DataList::Iterator it = node.children.begin();
QDBusConnectionPrivate::ObjectTreeNode::DataList::Iterator tgt = it;
for ( ; it != end; ++it) {
if (it->isActive())
*tgt++ = qMove(*it);
}
++tgt;
node.children.erase(tgt, end);
}
}
static void huntAndDestroy(QObject *needle, QDBusConnectionPrivate::ObjectTreeNode &haystack)
{
QDBusConnectionPrivate::ObjectTreeNode::DataList::Iterator it = haystack.children.begin();
QDBusConnectionPrivate::ObjectTreeNode::DataList::Iterator end = haystack.children.end();
for ( ; it != end; ++it) {
if (!it->isActive())
continue;
while (it != haystack.children.end()) {
huntAndDestroy(needle, *it);
if (!it->isActive())
--haystack.activeChildren;
it = haystack.children.erase(it);
else
it++;
}
if (needle == haystack.obj) {
haystack.obj = 0;
haystack.flags = 0;
}
garbageCollectChildren(haystack);
}
static void huntAndUnregister(const QStringList &pathComponents, int i, QDBusConnection::UnregisterMode mode,
@ -639,7 +615,6 @@ static void huntAndUnregister(const QStringList &pathComponents, int i, QDBusCon
if (mode == QDBusConnection::UnregisterTree) {
// clear the sub-tree as well
node->activeChildren = 0;
node->children.clear(); // can't disconnect the objects because we really don't know if they can
// be found somewhere else in the path too
}
@ -648,14 +623,12 @@ static void huntAndUnregister(const QStringList &pathComponents, int i, QDBusCon
QDBusConnectionPrivate::ObjectTreeNode::DataList::Iterator end = node->children.end();
QDBusConnectionPrivate::ObjectTreeNode::DataList::Iterator it =
std::lower_bound(node->children.begin(), end, pathComponents.at(i));
if (it == end || it->name != pathComponents.at(i) || !it->isActive())
if (it == end || it->name != pathComponents.at(i))
return; // node not found
huntAndUnregister(pathComponents, i + 1, mode, it);
if (!it->isActive())
--node->activeChildren;
garbageCollectChildren(*node);
node->children.erase(it);
}
}