Fixed memory leak on callbacks used on signal connection.

Now using the 'destroyed()' signal the reference is cleaned after source
object destroyed.

11 files changed, 208 insertions, 80 deletions

diff --git a/PySide/QtCore/glue/qobject_connect.cpp b/PySide/QtCore/glue/qobject_connect.cpp
index a8c856349..1b28ee0a8 100644
--- a/PySide/QtCore/glue/qobject_connect.cpp
+++ b/PySide/QtCore/glue/qobject_connect.cpp
@@ -1,3 +1,29 @@
+static bool getReceiver(PyObject *callback, QObject **receiver, PyObject **self)
+{
+    if (PyMethod_Check(callback)) {
+        *self = PyMethod_GET_SELF(callback);
+        if (SbkQObject_Check(*self))
+            *receiver = SbkQObject_cptr(*self);
+    } else if (PyCFunction_Check(callback)) {
+        *self = PyCFunction_GET_SELF(callback);
+        if (*self && SbkQObject_Check(*self))
+            *receiver = SbkQObject_cptr(*self);
+    } else if (!PyFunction_Check(callback)) {
+        *receiver = 0;
+        *self = 0;
+        qWarning() << "Invalid callback object.";
+        return false;
+    }
+
+    bool usingGlobalReceiver = !*receiver;
+    if (usingGlobalReceiver) {
+        PySide::SignalManager& signalManager = PySide::SignalManager::instance();
+        *receiver = signalManager.globalReceiver();
+    }
+
+    return usingGlobalReceiver;
+}
+
 static bool qobjectConnect(QObject* source, const char* signal, QObject* receiver, const char* slot, Qt::ConnectionType type)
 {
     if (!PySide::checkSignal(signal))
@@ -24,25 +50,11 @@ static bool qobjectConnectCallback(QObject* source, const char* signal, PyObject
     PySide::SignalManager& signalManager = PySide::SignalManager::instance();
 
     // Extract receiver from callback
-    bool usingGlobalReceiver;
     QObject* receiver = 0;
     PyObject* self = 0;
-    if (PyMethod_Check(callback)) {
-        self = PyMethod_GET_SELF(callback);
-        if (SbkQObject_Check(self))
-            receiver = SbkQObject_cptr(self);
-    } else if (PyCFunction_Check(callback)) {
-        self = PyCFunction_GET_SELF(callback);
-        if (self && SbkQObject_Check(self))
-            receiver = SbkQObject_cptr(self);
-    } else if (!PyFunction_Check(callback)) {
-        qWarning() << "Invalid callback object.";
+    bool usingGlobalReceiver = getReceiver(callback, &receiver, &self);
+    if (receiver == 0 and self == 0)
         return false;
-    }
-
-    usingGlobalReceiver = !receiver;
-    if (usingGlobalReceiver)
-        receiver = signalManager.globalReceiver();
 
     const QMetaObject* metaObject = receiver->metaObject();
     const QByteArray callbackSig = PySide::getCallbackSignature(signal, callback, usingGlobalReceiver).toAscii();
@@ -66,10 +78,10 @@ static bool qobjectConnectCallback(QObject* source, const char* signal, PyObject
         #ifndef AVOID_PROTECTED_HACK
             source->connectNotify(signal);
         #else
-            reinterpret_cast<QObjectWrapper*>(source)->connectNotify_protected(signal);
+            reinterpret_cast<QObjectWrapper*>(source)->connectNotify_protected(source, signal);
         #endif
         if (usingGlobalReceiver)
-            signalManager.globalReceiverConnectNotify(slotIndex);
+            signalManager.globalReceiverConnectNotify(source, slotIndex);
 
         return true;
     }
@@ -85,17 +97,11 @@ static bool qobjectDisconnectCallback(QObject* source, const char* signal, PyObj
     PySide::SignalManager& signalManager = PySide::SignalManager::instance();
 
     // Extract receiver from callback
-    bool usingGlobalReceiver;
     QObject* receiver = 0;
-    PyObject* self;
-    if (PyMethod_Check(callback)) {
-        self = PyMethod_GET_SELF(callback);
-        if (SbkQObject_Check(self))
-            receiver = SbkQObject_cptr(self);
-    }
-    usingGlobalReceiver = !receiver;
-    if (usingGlobalReceiver)
-        receiver = signalManager.globalReceiver();
+    PyObject* self = 0;
+    bool usingGlobalReceiver = getReceiver(callback, &receiver, &self);
+    if (receiver == 0 and self == 0)
+        return false;
 
     const QMetaObject* metaObject = receiver->metaObject();
     const QByteArray callbackSig = PySide::getCallbackSignature(signal, callback, usingGlobalReceiver).toAscii();
@@ -105,7 +111,7 @@ static bool qobjectDisconnectCallback(QObject* source, const char* signal, PyObj
     if (QObject::disconnect(source, signal, receiver, qtSlotName.constData())) {
         if (usingGlobalReceiver) {
             int slotIndex = metaObject->indexOfSlot(callbackSig.constData());
-            signalManager.globalReceiverDisconnectNotify(slotIndex);
+            signalManager.globalReceiverDisconnectNotify(source, slotIndex);
         }
         return true;
     }
diff --git a/PySide/QtCore/typesystem_core.xml b/PySide/QtCore/typesystem_core.xml
index fdfeb3739..212e19226 100644
--- a/PySide/QtCore/typesystem_core.xml
+++ b/PySide/QtCore/typesystem_core.xml
@@ -1631,6 +1631,16 @@
         </inject-code>
     </add-function>
 
+   <modify-function signature="receivers(const char*) const">
+     <inject-code class="target" position="beginning">
+       //Avoid return +1 because SignalManager connect to "destroyed()" signal to control object timelife
+       int ret = %CPPSELF.%FUNCTION_NAME(%1);
+       if ((strcmp(%1, SIGNAL(destroyed())) == 0) &amp;&amp; (PySide::SignalManager::instance().hasConnectionWith(%CPPSELF)))
+         ret--;
+       %PYARG_0 = %CONVERTTOPYTHON[int](ret);
+     </inject-code>
+   </modify-function>
+
    <modify-function signature="sender() const">
       <modify-argument index="return">
          <define-ownership owner="target"/>
diff --git a/libpyside/dynamicqmetaobject.cpp b/libpyside/dynamicqmetaobject.cpp
index 192b9bcc8..dbc51803c 100644
--- a/libpyside/dynamicqmetaobject.cpp
+++ b/libpyside/dynamicqmetaobject.cpp
@@ -58,6 +58,13 @@ static int registerString(const QByteArray& s, QList<QByteArray>* strings)
     return idx;
 }
 
+static void clearItem(QLinkedList<QByteArray> &l, const QByteArray &value)
+{
+    QLinkedList<QByteArray>::iterator i = qFind(l.begin(), l.end(), value);
+    if (i != l.end())
+        *i = QByteArray();
+}
+
 DynamicQMetaObject::DynamicQMetaObject(const char *className, const QMetaObject* metaObject)
 {
     d.superdata = metaObject;
@@ -76,6 +83,15 @@ DynamicQMetaObject::~DynamicQMetaObject()
 
 void DynamicQMetaObject::addSignal(const char* signal)
 {
+    //search for a empty space
+    QByteArray blank;
+    QLinkedList<QByteArray>::iterator i = qFind(m_signals.begin(), m_signals.end(), blank);
+    if (i != m_signals.end()) {
+        *i = QByteArray(signal);
+        updateMetaObject();
+        return;
+    }
+
     if (m_signals.size() >= MAX_SIGNALS_COUNT) {
         qWarning() << "Fail to add dynamic signal to QObject. PySide support at most" << MAX_SIGNALS_COUNT << "dynamic signals.";
         return;
@@ -87,25 +103,34 @@ void DynamicQMetaObject::addSignal(const char* signal)
 
 void DynamicQMetaObject::addSlot(const char* slot)
 {
-    m_slots << QByteArray(slot);
+    //search for a empty space
+    QByteArray blank;
+    QLinkedList<QByteArray>::iterator i = qFind(m_slots.begin(), m_slots.end(), blank);
+    if (i != m_slots.end()) {
+        *i = QByteArray(slot);
+    } else {
+        m_slots << QByteArray(slot);
+    }
     updateMetaObject();
 }
 
 void DynamicQMetaObject::removeSlot(uint index)
 {
     QMetaMethod m = method(index);
-    if (m_slots.removeAll(m.signature()))
+    if (m_slots.contains(m.signature())) {
+        clearItem(m_slots, m.signature());
         updateMetaObject();
+    }
 }
 
 void DynamicQMetaObject::removeSignal(uint index)
 {
     //Current Qt implementation does not support runtime remove signal
-    /*
     QMetaMethod m = method(index);
-    if (m_signals.removeAll(m.signature()))
+    if (m_signals.contains(m.signature())) {
+        clearItem(m_signals, m.signature());
         updateMetaObject();
-    */
+    }
 }
 
 void DynamicQMetaObject::updateMetaObject()
diff --git a/libpyside/globalreceiver.cpp b/libpyside/globalreceiver.cpp
index 45373f557..0cae075be 100644
--- a/libpyside/globalreceiver.cpp
+++ b/libpyside/globalreceiver.cpp
@@ -32,94 +32,125 @@
 * 02110-1301 USA
 */
 
-#include "globalreceiver.h"
 #include <QMetaMethod>
 #include <QDebug>
-#include "signalmanager.h"
+#include <QEvent>
 #include <autodecref.h>
 #include <gilstate.h>
+
+#include "globalreceiver.h"
 #include "typeresolver.h"
+#include "signalmanager.h"
+#include "weakref.h"
 
+#define RECEIVER_DESTROYED_SLOT_NAME "__receiverDestroyed__(QObject*)"
 
 namespace PySide
 {
-
 class DynamicSlotData
 {
     public:
-        DynamicSlotData(PyObject *callback);
-        void incRef();
-        void decRef();
+        DynamicSlotData(int id, PyObject* callback);
+        void addRef(const QObject* o);
+        void decRef(const QObject* o);
+        void clear();
+        bool hasRefTo(const QObject* o) const;
         int refCount() const;
-        PyObject *callback() const;
+        int id() const;
+        PyObject* callback() const;
         ~DynamicSlotData();
 
     private:
-        int m_refCount;
-        PyObject *m_callback;
+        int m_id;
+        PyObject* m_callback;
+        QSet<const QObject*> m_refs;
 };
 
 }
 
 using namespace PySide;
 
-DynamicSlotData::DynamicSlotData(PyObject *callback)
-    : m_refCount(0)
+DynamicSlotData::DynamicSlotData(int id, PyObject* callback)
+    : m_id(id)
 {
     m_callback = callback;
     Py_INCREF(callback);
 }
 
-void DynamicSlotData::incRef()
+void DynamicSlotData::addRef(const QObject *o)
 {
-    m_refCount++;
+    m_refs.insert(o);
 }
 
-void DynamicSlotData::decRef()
+void DynamicSlotData::decRef(const QObject *o)
 {
-    m_refCount--;
+    m_refs.remove(o);
 }
 
 int DynamicSlotData::refCount() const
 {
-    return m_refCount;
+    return m_refs.size();
 }
 
-PyObject *DynamicSlotData::callback() const
+
+PyObject* DynamicSlotData::callback() const
 {
     return m_callback;
 }
 
+int DynamicSlotData::id() const
+{
+    return m_id;
+}
+
+bool DynamicSlotData::hasRefTo(const QObject *o) const
+{
+    return m_refs.contains(o);
+}
+
+void DynamicSlotData::clear()
+{
+    m_refs.clear();
+}
+
 DynamicSlotData::~DynamicSlotData()
 {
-    Py_XDECREF(m_callback);
+    Py_DECREF(m_callback);
 }
 
 
 GlobalReceiver::GlobalReceiver()
     : m_metaObject("GlobalReceiver", &QObject::staticMetaObject)
 {
+    //slot used to be notifyed of object destrouction
+    m_metaObject.addSlot(RECEIVER_DESTROYED_SLOT_NAME);
 }
 
 GlobalReceiver::~GlobalReceiver()
 {
-    foreach(DynamicSlotData* data, m_slotReceivers)
+    foreach(DynamicSlotData* data, m_slotReceivers) {
+        data->clear();
         delete data;
+    }
 }
 
-void GlobalReceiver::connectNotify(int slotId)
+void GlobalReceiver::connectNotify(QObject* source, int slotId)
 {
-    if (m_slotReceivers.contains(slotId))
-        m_slotReceivers[slotId]->incRef();
+    if (m_slotReceivers.contains(slotId)) {
+        m_slotReceivers[slotId]->addRef(source);
+    }
 }
 
-void GlobalReceiver::disconnectNotify(int slotId)
+void GlobalReceiver::disconnectNotify(QObject* source, int slotId)
 {
     if (m_slotReceivers.contains(slotId)) {
+        QObject::disconnect(source, SIGNAL(destroyed(QObject*)), this, "1"RECEIVER_DESTROYED_SLOT_NAME);
+
         DynamicSlotData *data = m_slotReceivers[slotId];
-        data->decRef();
-        if (data->refCount() == 0)
+        data->decRef(source);
+        if (data->refCount() == 0) {
             removeSlot(slotId);
+        }
     }
 }
 
@@ -133,7 +164,7 @@ void GlobalReceiver::addSlot(const char* slot, PyObject* callback)
     m_metaObject.addSlot(slot);
     int slotId = m_metaObject.indexOfSlot(slot);
     if (!m_slotReceivers.contains(slotId)) {
-        m_slotReceivers[slotId] = new DynamicSlotData(callback);
+        m_slotReceivers[slotId] = new DynamicSlotData(slotId, callback);
     }
 
     bool isShortCircuit = true;
@@ -143,6 +174,7 @@ void GlobalReceiver::addSlot(const char* slot, PyObject* callback)
             break;
         }
     }
+
     if (isShortCircuit)
         m_shortCircuitSlots << slotId;
 
@@ -151,9 +183,23 @@ void GlobalReceiver::addSlot(const char* slot, PyObject* callback)
 
 void GlobalReceiver::removeSlot(int slotId)
 {
-    delete m_slotReceivers.take(slotId);
-    m_metaObject.removeSlot(slotId);
-    m_shortCircuitSlots.remove(slotId);
+    if (m_slotReceivers.contains(slotId)) {
+        delete m_slotReceivers.take(slotId);
+        m_metaObject.removeSlot(slotId);
+        m_shortCircuitSlots.remove(slotId);
+    }
+}
+
+bool GlobalReceiver::hasConnectionWith(const QObject *object)
+{
+    QHash<int, DynamicSlotData*>::iterator i = m_slotReceivers.begin();
+    while(i != m_slotReceivers.end()) {
+        if (i.value()->hasRefTo(object)) {
+            return true;
+        }
+        i++;
+    }
+    return false;
 }
 
 int GlobalReceiver::qt_metacall(QMetaObject::Call call, int id, void** args)
@@ -163,6 +209,20 @@ int GlobalReceiver::qt_metacall(QMetaObject::Call call, int id, void** args)
     QMetaMethod slot = m_metaObject.method(id);
     Q_ASSERT(slot.methodType() == QMetaMethod::Slot);
 
+    if (strcmp(slot.signature(), RECEIVER_DESTROYED_SLOT_NAME) == 0) {
+        QObject *arg = *(QObject**)args[1];
+
+        QHash<int, DynamicSlotData*>::iterator i = m_slotReceivers.begin();
+        while(i != m_slotReceivers.end()) {
+            if (i.value()->hasRefTo(arg)) {
+                disconnectNotify(arg, i.key());
+                break;
+            }
+            i++;
+        }
+        return -1;
+    }
+
     DynamicSlotData* data = m_slotReceivers.value(id);
     if (!data) {
         qWarning() << "Unknown global slot, id:" << id;
@@ -191,5 +251,6 @@ int GlobalReceiver::qt_metacall(QMetaObject::Call call, int id, void** args)
         qWarning() << "Error calling slot" << m_metaObject.method(id).signature();
     else
         Py_DECREF(retval);
+
     return -1;
 }
diff --git a/libpyside/globalreceiver.h b/libpyside/globalreceiver.h
index 9243f9d3f..605003603 100644
--- a/libpyside/globalreceiver.h
+++ b/libpyside/globalreceiver.h
@@ -55,8 +55,10 @@ public:
     const QMetaObject* metaObject() const;
     void addSlot(const char* slot, PyObject* callback);
     void removeSlot(int slotId);
-    void connectNotify(int slotId);
-    void disconnectNotify(int slotId);
+    void connectNotify(QObject* sender, int slotId);
+    void disconnectNotify(QObject* sender, int slotId);
+    bool hasConnectionWith(const QObject* object);
+
 private:
     DynamicQMetaObject m_metaObject;
     QSet<int> m_shortCircuitSlots;
diff --git a/libpyside/signalmanager.cpp b/libpyside/signalmanager.cpp
index 7a3236fd1..9d648611d 100644
--- a/libpyside/signalmanager.cpp
+++ b/libpyside/signalmanager.cpp
@@ -69,7 +69,12 @@ bool PySide::checkSignal(const char* signal)
 
 static QString codeCallbackName(PyObject* callback, const QString& funcName)
 {
-    return funcName+QString::number(quint64(callback), 16);
+    if (PyMethod_Check(callback)) {
+        PyObject *self = PyMethod_GET_SELF(callback);
+        PyObject *func = PyMethod_GET_FUNCTION(callback);
+        return funcName + QString::number(quint64(self), 16) + QString::number(quint64(func), 16);
+    } else
+        return funcName+QString::number(quint64(callback), 16);
 }
 
 QString PySide::getCallbackSignature(const char* signal, PyObject* callback, bool encodeName)
@@ -185,14 +190,14 @@ QObject* SignalManager::globalReceiver()
     return &m_d->m_globalReceiver;
 }
 
-void SignalManager::globalReceiverConnectNotify(int slotIndex)
+void SignalManager::globalReceiverConnectNotify(QObject* source, int slotIndex)
 {
-    m_d->m_globalReceiver.connectNotify(slotIndex);
+    m_d->m_globalReceiver.connectNotify(source, slotIndex);
 }
 
-void SignalManager::globalReceiverDisconnectNotify(int slotIndex)
+void SignalManager::globalReceiverDisconnectNotify(QObject* source, int slotIndex)
 {
-    m_d->m_globalReceiver.disconnectNotify(slotIndex);
+    m_d->m_globalReceiver.disconnectNotify(source, slotIndex);
 }
 
 void SignalManager::addGlobalSlot(const char* slot, PyObject* callback)
@@ -234,7 +239,6 @@ bool SignalManager::emitSignal(QObject* source, const char* signal, PyObject* ar
         return false;
     signal++;
 
-
     int signalIndex = source->metaObject()->indexOfSignal(signal);
     if (signalIndex != -1) {
         bool isShortCircuit;
@@ -311,3 +315,8 @@ bool SignalManager::registerMetaMethod(QObject* source, const char* signature, Q
     }
     return true;
 }
+
+bool SignalManager::hasConnectionWith(const QObject *object)
+{
+    return m_d->m_globalReceiver.hasConnectionWith(object);
+}
diff --git a/libpyside/signalmanager.h b/libpyside/signalmanager.h
index 67031a6c7..02d760ddc 100644
--- a/libpyside/signalmanager.h
+++ b/libpyside/signalmanager.h
@@ -61,11 +61,14 @@ public:
     static int qt_metacall(QObject* object, QMetaObject::Call call, int id, void** args);
     void addGlobalSlot(const char* slot, PyObject* callback);
 
-    void globalReceiverConnectNotify(int slotIndex);
-    void globalReceiverDisconnectNotify(int slotIndex);
+    void globalReceiverConnectNotify(QObject *sender, int slotIndex);
+    void globalReceiverDisconnectNotify(QObject *sender, int slotIndex);
 
-    // Used to register a new signal/slot on QMetaobjc of source.
+    // Used to register a new signal/slot on QMetaobject of source.
     static bool registerMetaMethod(QObject* source, const char* signature, QMetaMethod::MethodType type);
+
+    // Used to discovery if SignalManager was connected with object "destroyed()" signal.
+    bool hasConnectionWith(const QObject *object);
 private:
     struct SignalManagerPrivate;
     SignalManagerPrivate* m_d;
diff --git a/tests/qtcore/qobject_protected_methods_test.py b/tests/qtcore/qobject_protected_methods_test.py
index 37bda37bc..530b870a2 100644
--- a/tests/qtcore/qobject_protected_methods_test.py
+++ b/tests/qtcore/qobject_protected_methods_test.py
@@ -23,7 +23,7 @@ class QObjectReceivers(unittest.TestCase):
         self.assertEqual(obj.receivers(SIGNAL("destroyed()")), 0)
 
         QObject.connect(obj, SIGNAL("destroyed()"), self.cb)
-        self.assertEqual(obj.receivers(SIGNAL("destroyed()")), 1)
+        self.assertEqual(obj.receivers(SIGNAL("destroyed()")), 0)
 
     def testQThreadReceiversExtern(self):
         #QThread.receivers() - Inherited protected method
@@ -31,7 +31,7 @@ class QObjectReceivers(unittest.TestCase):
         obj = QThread()
         self.assertEqual(obj.receivers(SIGNAL('destroyed()')), 0)
         QObject.connect(obj, SIGNAL("destroyed()"), self.cb)
-        self.assertEqual(obj.receivers(SIGNAL("destroyed()")), 1)
+        self.assertEqual(obj.receivers(SIGNAL("destroyed()")), 0)
 
 
 if __name__ == '__main__':
diff --git a/tests/signals/multiple_connections_test.py b/tests/signals/multiple_connections_test.py
index d2a207673..9f45f4da4 100644
--- a/tests/signals/multiple_connections_test.py
+++ b/tests/signals/multiple_connections_test.py
@@ -10,7 +10,7 @@ from helper import BasicPySlotCase, UsesQCoreApplication
 from helper.decorators import requires
 
 
-def random_gen(count=100, largest=99, lowest=0):
+def random_gen(count=50, largest=49, lowest=0):
     for i in range(count):
         yield random.randint(lowest, largest)
 
@@ -50,7 +50,7 @@ class PythonMultipleSlots(UsesQCoreApplication, MultipleSignalConnections):
         class Dummy(QObject):
             pass
 
-        for test in random_gen(30):
+        for test in random_gen(20):
             sender = Dummy()
             receivers = [BasicPySlotCase() for x in range(10)]
             self.run_many(sender, 'foobar', partial(sender.emit,
diff --git a/tests/signals/pysignal_test.py b/tests/signals/pysignal_test.py
index 3d741dcca..639a7dee2 100644
--- a/tests/signals/pysignal_test.py
+++ b/tests/signals/pysignal_test.py
@@ -50,6 +50,18 @@ class PythonSigSlot(unittest.TestCase):
         self.assert_(self.called)
 
 
+    def testDisconnect(self):
+        obj1 = Dummy()
+
+        QObject.connect(obj1, SIGNAL('foo(int)'), self.callback)
+        QObject.disconnect(obj1, SIGNAL('foo(int)'), self.callback)
+
+        self.args = (42, )
+        obj1.emit(SIGNAL('foo(int)'), *self.args)
+
+        self.assert_(not self.called)
+
+
 @requires('PySide.QtGui')
 class SpinBoxPySignal(UsesQApplication):
     """Tests the connection of python signals to QSpinBox qt slots."""
diff --git a/tests/signals/qobject_receivers_test.py b/tests/signals/qobject_receivers_test.py
index e1d429ec6..cb4c9ced8 100644
--- a/tests/signals/qobject_receivers_test.py
+++ b/tests/signals/qobject_receivers_test.py
@@ -30,9 +30,9 @@ class TestQObjectReceivers(unittest.TestCase):
         sender = QObject()
         receiver = QObject()
         sender.connect(sender, SIGNAL("destroyed()"), cute_slot)
-        self.assertEqual(sender.receivers(SIGNAL("destroyed(  )")), 1)
+        self.assertEqual(sender.receivers(SIGNAL("destroyed(  )")), 0)
         sender.connect(sender, SIGNAL("destroyed()"), receiver, SLOT("deleteLater()"))
-        self.assertEqual(sender.receivers(SIGNAL("destroyed()")), 2)
+        self.assertEqual(sender.receivers(SIGNAL("destroyed()")), 1)
         del sender
         del receiver