Fixed a potential infinite recursion for Qt bindings

The problem is caused by an implementation detail:
To monitor the lifetime of Qt objects, a monitoring
object is attached to them. This used to happen through
child objects but attaching such issues QChildEvents
which - if routed over script objects - will itself
required objects to be attached a monitoring object.

The solution is to attach monitoring objects through
QObject dynamic properties.
This commit is contained in:
Matthias Koefferlein 2017-07-26 23:27:24 +02:00
parent c442202ca4
commit b13c2e69c3
5 changed files with 92 additions and 25 deletions

View File

@ -91,7 +91,7 @@ Proxy::destroy ()
void
Proxy::detach ()
{
if (m_cls_decl && m_cls_decl->is_managed ()) {
if (! m_destroyed && m_cls_decl && m_cls_decl->is_managed ()) {
gsi::ObjectBase *gsi_object = m_cls_decl->gsi_object (m_obj, false);
if (gsi_object) {
gsi_object->status_changed_event ().remove (this, &Proxy::object_status_changed);
@ -217,6 +217,7 @@ void
Proxy::object_status_changed (gsi::ObjectBase::StatusEventType type)
{
if (type == gsi::ObjectBase::ObjectDestroyed) {
m_destroyed = true; // NOTE: must be set before detach and indicates that the object was destroyed externally.
detach ();
} else if (type == gsi::ObjectBase::ObjectKeep) {
m_owned = false;

View File

@ -21,6 +21,73 @@
*/
#include "gsiQtHelper.h"
#include "tlObject.h"
#include <QMetaType>
namespace qt_gsi
{
/**
* @brief A tiny object being both a gsi::ObjectBase for lifetime monitoring and tl::Object for shared pointer management
*
* See QtWatcher for details.
*/
class QtLifetimeMonitor
: public tl::Object, public gsi::ObjectBase
{
// .. nothing yet ..
};
/**
* @brief A helper object that is attached to a QObject to monitor it's lifetime
*
* This object will be put into the properties table of the QObject (more precisely: a copy).
* When the QObject is destroyed, it's properties are destroyed too and through reference
* counting the destruction of the QObject is detected. The monitoring itself is implemented
* through a gsi::ObjectBase object which plugs seamlessly into the gsi type system:
*
* QObject -> QtWatcher (+ temp copies) -> gsi::ObjectBase -> plugs into script objects
*
* NOTE: the initial idea was to use child objects attached to QObject. However, these
* interfere too much with Qt's internals - for example, attaching a child object emits
* a QChildEvent. That gets filtered in some eventFilter function which itself may cause
* QObjects to be decorated with helper objects ... resulting in a infinte recursion.
*
* Decoration by properties is a leaner solution.
*/
class QtWatcher
{
public:
QtWatcher ()
{
// .. nothing yet ..
}
QtWatcher (QtLifetimeMonitor *monitor)
: mp_monitor (monitor)
{
// .. nothing yet ..
}
gsi::ObjectBase *gsi_object ()
{
if (mp_monitor) {
return static_cast<gsi::ObjectBase *> (mp_monitor.get ());
} else {
return 0;
}
}
private:
QtWatcher (size_t id, gsi::ObjectBase *obj);
tl::shared_ptr<QtLifetimeMonitor> mp_monitor;
};
}
Q_DECLARE_METATYPE (qt_gsi::QtWatcher)
namespace qt_gsi
{
@ -30,11 +97,24 @@ namespace qt_gsi
*/
gsi::ObjectBase *get_watcher_object (QObject *qobject, bool required)
{
QtWatcherObject *watcher = qobject->findChild<QtWatcherObject *> ();
if (! watcher && required) {
watcher = new QtWatcherObject (qobject);
const char *watcher_prop_name = "_gsi_qt::watcher";
QVariant prop = qobject->property (watcher_prop_name);
if (prop.isValid ()) {
return prop.value<QtWatcher> ().gsi_object ();
} else if (required) {
QtWatcher w (new QtLifetimeMonitor ());
qobject->setProperty (watcher_prop_name, QVariant::fromValue<QtWatcher> (w));
return w.gsi_object ();
} else {
return 0;
}
return watcher;
}
}

View File

@ -32,23 +32,6 @@ namespace qt_gsi
gsi::ObjectBase *get_watcher_object (QObject *qobject, bool required);
/**
* @brief A tiny watcher object linking QObject and gsi::ObjectBase
* This object is hooked into the watched object's child list to watch it's lifetime.
*/
class QtWatcherObject
: public QObject, public gsi::ObjectBase
{
Q_OBJECT
public:
QtWatcherObject (QObject *parent)
: QObject (parent), gsi::ObjectBase ()
{
// .. nothing yet ..
}
};
}
#endif

View File

@ -347,6 +347,7 @@ PYAObjectBase::~PYAObjectBase ()
} catch (...) {
tl::warn << "Caught unspecified exception in object destructor";
}
m_destroyed = true;
}
@ -362,8 +363,9 @@ PYAObjectBase::object_status_changed (gsi::ObjectBase::StatusEventType type)
bool prev_owner = m_owned;
m_destroyed = true; // NOTE: must be set before detach!
detach ();
m_destroyed = true;
// NOTE: this may delete "this"!
if (!prev_owner) {
@ -432,7 +434,7 @@ PYAObjectBase::detach ()
const gsi::ClassBase *cls = cls_decl ();
if (cls && cls->is_managed ()) {
if (! m_destroyed && cls && cls->is_managed ()) {
gsi::ObjectBase *gsi_object = cls->gsi_object (m_obj, false);
if (gsi_object) {
gsi_object->status_changed_event ().remove (&m_listener, &StatusChangedListener::object_status_changed);

View File

@ -379,7 +379,7 @@ Proxy::destroy ()
void
Proxy::detach ()
{
if (m_cls_decl && m_cls_decl->is_managed ()) {
if (! m_destroyed && m_cls_decl && m_cls_decl->is_managed ()) {
gsi::ObjectBase *gsi_object = m_cls_decl->gsi_object (m_obj, false);
if (gsi_object) {
gsi_object->status_changed_event ().remove (this, &Proxy::object_status_changed);
@ -684,6 +684,7 @@ void
Proxy::object_status_changed (gsi::ObjectBase::StatusEventType type)
{
if (type == gsi::ObjectBase::ObjectDestroyed) {
m_destroyed = true; // NOTE: must be set before detach and indicates that the object was destroyed externally.
detach ();
} else if (type == gsi::ObjectBase::ObjectKeep) {
keep_internal ();