Fix Segfault when using connect (w/o instrumentation)

The multiple problems were caused by wrong or missing refcounts.
They led probably to the assumption that we need the garbage collector.

There is no garbage collector needed, since there exist no cyclic references.
Some increfs and decrefs were not correct, and a few special rules concerning
reference counting were ignored.

This is a final solution:

The case of a function call that returns a signal is special.
In this case, we keep track of the extra reference in a structure.
In all other cases there is a pre-existing object.

This patch is not creating any other errors.
It is no longer a hack but a true solution.

Added explicit path setting in order to circumvent the spurious python 2.7
error in disconnect_test.py .

Task-number: PYSIDE-79
Change-Id: I2579adf8fc671a2c2b665cfddaa26ecf96300a99
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

6 files changed, 89 insertions, 3 deletions

diff --git a/libpyside/pysideproperty.cpp b/libpyside/pysideproperty.cpp
index 03415f16..3c335d6f 100644
--- a/libpyside/pysideproperty.cpp
+++ b/libpyside/pysideproperty.cpp
@@ -339,7 +339,12 @@ static PyObject* getFromType(PyTypeObject* type, PyObject* name)
             if (attr)
                 return attr;
         }
+        // PYSIDE-79: needed to capture this code path - attr not found
+        return PyErr_Format(PyExc_RuntimeError, "*** Attribute '%s' not found!",
+                            Shiboken::String::toCString(name));
     }
+    Py_INCREF(attr); // PYSIDE-79: missing incref. PyDict_GetItem borrows a ref.
+    // This was not central to the error, but caused sometimes later crashes.
     return attr;
 }
 
diff --git a/libpyside/pysidesignal.cpp b/libpyside/pysidesignal.cpp
index 51c17568..a9feb63d 100644
--- a/libpyside/pysidesignal.cpp
+++ b/libpyside/pysidesignal.cpp
@@ -406,13 +406,17 @@ PyObject* signalInstanceConnect(PyObject* self, PyObject* args, PyObject* kwds)
     if (match) {
         Shiboken::AutoDecRef tupleArgs(PyList_AsTuple(pyArgs));
         Shiboken::AutoDecRef pyMethod(PyObject_GetAttrString(source->d->source, "connect"));
+        if (pyMethod == 0) { // PYSIDE-79: check if pyMethod exists.
+            PyErr_SetString(PyExc_RuntimeError, "method 'connect' vanished!");
+            return 0;
+        }
         PyObject* result = PyObject_CallObject(pyMethod, tupleArgs);
         if (result == Py_True)
             return result;
         else
             Py_XDECREF(result);
     }
-    if (PyErr_Occurred())
+    if (!PyErr_Occurred()) // PYSIDE-79: inverse the logic. A Null return needs an error.
         PyErr_Format(PyExc_RuntimeError, "Failed to connect signal %s.", source->d->signature);
     return 0;
 }
@@ -677,6 +681,9 @@ void appendSignature(PySideSignal* self, char* signature)
 PySideSignalInstance* initialize(PySideSignal* self, PyObject* name, PyObject* object)
 {
     PySideSignalInstance* instance = PyObject_New(PySideSignalInstance, &PySideSignalInstanceType);
+    SbkObject* sbkObj = reinterpret_cast<SbkObject*>(object);
+    if (!Shiboken::Object::wasCreatedByPython(sbkObj))
+        Py_INCREF(object); // PYSIDE-79: this flag was crucial for a wrapper call.
     instanceInitialize(instance, name, self, object, 0);
     return instance;
 }
@@ -740,6 +747,7 @@ PySideSignalInstance* newObjectFromMethod(PyObject* source, const QList<QMetaMet
         item->d = new PySideSignalInstancePrivate;
         PySideSignalInstancePrivate* selfPvt = item->d;
         selfPvt->source = source;
+        Py_INCREF(selfPvt->source); // PYSIDE-79: an INCREF is missing.
         QByteArray cppName(m.methodSignature());
         cppName.truncate(cppName.indexOf('('));
         // separe SignalName
diff --git a/libpyside/pysideweakref.cpp b/libpyside/pysideweakref.cpp
index e44ef3d2..3223d06b 100644
--- a/libpyside/pysideweakref.cpp
+++ b/libpyside/pysideweakref.cpp
@@ -130,10 +130,10 @@ PyObject* create(PyObject* obj, PySideWeakRefFunction func, void* userData)
     if (!weak || PyErr_Occurred())
         return 0;
 
-    Py_DECREF(callable);
-
     callable->weakref_func = func;
     callable->user_data = userData;
+    Py_DECREF(callable); // PYSIDE-79: after decref the callable is undefined (theoretically)
+
     return (PyObject*)weak;
 }
 
diff --git a/tests/signals/CMakeLists.txt b/tests/signals/CMakeLists.txt
index 12105cb8..fe06a343 100644
--- a/tests/signals/CMakeLists.txt
+++ b/tests/signals/CMakeLists.txt
@@ -1,4 +1,5 @@
 PYSIDE_TEST(args_dont_match_test.py)
+PYSIDE_TEST(bug_79.py)
 PYSIDE_TEST(bug_189.py)
 PYSIDE_TEST(bug_311.py)
 PYSIDE_TEST(bug_312.py)
diff --git a/tests/signals/bug_79.py b/tests/signals/bug_79.py
new file mode 100644
index 00000000..0174276e
--- /dev/null
+++ b/tests/signals/bug_79.py
@@ -0,0 +1,69 @@
+#############################################################################
+##
+## Copyright (C) 2016 The Qt Company Ltd.
+## Contact: https://www.qt.io/licensing/
+##
+## This file is part of the test suite of PySide2.
+##
+## $QT_BEGIN_LICENSE:GPL-EXCEPT$
+## Commercial License Usage
+## Licensees holding valid commercial Qt licenses may use this file in
+## accordance with the commercial license agreement provided with the
+## Software or, alternatively, in accordance with the terms contained in
+## a written agreement between you and The Qt Company. For licensing terms
+## and conditions see https://www.qt.io/terms-conditions. For further
+## information use the contact form at https://www.qt.io/contact-us.
+##
+## GNU General Public License Usage
+## Alternatively, this file may be used under the terms of the GNU
+## General Public License version 3 as published by the Free Software
+## Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+## included in the packaging of this file. Please review the following
+## information to ensure the GNU General Public License requirements will
+## be met: https://www.gnu.org/licenses/gpl-3.0.html.
+##
+## $QT_END_LICENSE$
+##
+#############################################################################
+
+from __future__ import print_function
+import unittest
+import sys
+import gc
+
+from PySide2 import QtGui, QtWidgets
+
+try:
+    from sys import gettotalrefcount
+    skiptest = False
+except ImportError:
+    skiptest = True
+
+class ConnectTest(unittest.TestCase):
+
+    def callback(self, o):
+        print("callback")
+        self._called = o
+
+    def testNoLeaks_ConnectAndDisconnect(self):
+        self._called = None
+        app = QtWidgets.QApplication([])
+        o = QtWidgets.QTreeView()
+        o.setModel(QtGui.QStandardItemModel())
+        o.selectionModel().destroyed.connect(self.callback)
+        o.selectionModel().destroyed.disconnect(self.callback)
+        gc.collect()
+        # if this is no debug build, then we check at least that
+        # we do not crash any longer.
+        if not skiptest:
+            total = sys.gettotalrefcount()
+        for idx in range(1000):
+            o.selectionModel().destroyed.connect(self.callback)
+            o.selectionModel().destroyed.disconnect(self.callback)
+        gc.collect()
+        if not skiptest:
+            self.assert_(abs(gettotalrefcount() - total) < 10)
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/signals/disconnect_test.py b/tests/signals/disconnect_test.py
index d4c1231f..44a79bbb 100644
--- a/tests/signals/disconnect_test.py
+++ b/tests/signals/disconnect_test.py
@@ -26,6 +26,9 @@
 ##
 #############################################################################
 
+import os, sys
+sys.path.insert(0, os.path.join("..", "pysidetest"))
+
 import unittest
 from PySide2.QtCore import *
 from testbinding import TestObject