Remember any signals that are connected to Python objects and disconnect them when the script is unloaded so the references to those objects can be dropped

3rdparty/pythonqt/src/PythonQt.h

@@ -453,6 +453,10 @@ signals:
   //! emitted when help() is called on a PythonQt object and \c ExternalHelp is enabled
   void pythonHelpRequest(const QByteArray& cppClassName);
 
+  //! emitted when a signal is connected to a Python object
+  void signalConnectedToPython(PythonQtSignalReceiver* receiver, int signal_id,
+                               PyObject* callable);
+
 private:
   void initPythonQtModule(bool redirectStdOut, const QByteArray& pythonQtModuleName);
 
@@ -472,6 +476,7 @@ private:
 
   PythonQtPrivate* _p;
 
+  friend class PythonQtSignalReceiver;
 };
 
 //! internal PythonQt details

3rdparty/pythonqt/src/PythonQtSignalReceiver.cpp

@@ -170,22 +170,26 @@ bool PythonQtSignalReceiver::addSignalHandler(const char* signal, PyObject* call
 
     _slotCount++;
     flag = true;
+
+    PythonQt::self()->signalConnectedToPython(this, sigId, callable);
   }
   return flag;
 }
 
 bool PythonQtSignalReceiver::removeSignalHandler(const char* signal, PyObject* callable)
+{
+  return removeSignalHandler(getSignalIndex(signal), callable);
+}
+
+bool PythonQtSignalReceiver::removeSignalHandler(int sigId, PyObject* callable)
 {
   bool found = false;
-  int sigId = getSignalIndex(signal);
-  if (sigId>=0) {
-    QMutableListIterator<PythonQtSignalTarget> i(_targets);
-    while (i.hasNext()) {
-      if (i.next().isSame(sigId, callable)) {
-        i.remove();
-        found = true;
-        break;
-      }
+  QMutableListIterator<PythonQtSignalTarget> i(_targets);
+  while (i.hasNext()) {
+    if (i.next().isSame(sigId, callable)) {
+      i.remove();
+      found = true;
+      break;
     }
   }
   return found;

3rdparty/pythonqt/src/PythonQtSignalReceiver.h

@@ -119,6 +119,7 @@ public:
 
   //! remove a signal handler
   bool removeSignalHandler(const char* signal, PyObject* callable);
+  bool removeSignalHandler(int sigId, PyObject* callable);
 
   //! remove all signal handlers
   void removeSignalHandlers();

src/scripting/python/pythonengine.cpp

@@ -16,6 +16,8 @@
 */
 
 #include <Python.h>
+#include <frameobject.h>
+
 #include <PythonQtConversion.h>
 #include <com_trolltech_qt_core/com_trolltech_qt_core_init.h>
 #include <com_trolltech_qt_gui/com_trolltech_qt_gui_init.h>
@@ -116,6 +118,9 @@ bool PythonEngine::EnsureInitialised() {
   connect(python_qt, SIGNAL(pythonStdOut(QString)), SLOT(PythonStdOut(QString)));
   connect(python_qt, SIGNAL(pythonStdErr(QString)), SLOT(PythonStdErr(QString)));
 
+  connect(python_qt, SIGNAL(signalConnectedToPython(PythonQtSignalReceiver*,int,PyObject*)),
+          SLOT(SignalConnectedToPython(PythonQtSignalReceiver*,int,PyObject*)));
+
   // Create a clementine module
   clementine_module_ = python_qt->createModuleFromScript(kClementineModuleName);
   PythonQt_init_Clementine(clementine_module_);
@@ -229,3 +234,43 @@ void PythonEngine::RemoveModuleFromModel(const QString& name) {
     modules_model_->removeRow(item->row());
   }
 }
+
+QString PythonEngine::CurrentScriptName() {
+  // Walk up the Python stack and find the name of the script that's nearest
+  // the top of the stack.
+  const QString prefix = QString(kScriptModulePrefix) + ".";
+
+  PyFrameObject* frame = PyEval_GetFrame();
+  while (frame) {
+    if (PyDict_Check(frame->f_globals)) {
+      PyObject* __name__ = PyDict_GetItemString(frame->f_globals, "__name__");
+      if (__name__ && PyString_Check(__name__)) {
+        const QString name(PyString_AsString(__name__));
+        if (name.startsWith(prefix)) {
+          int n = name.indexOf('.', prefix.length());
+          if (n != -1) {
+            n -= prefix.length();
+          }
+          return name.mid(prefix.length(), n);
+        }
+      }
+    }
+    frame = frame->f_back;
+  }
+  return QString();
+}
+
+Script* PythonEngine::CurrentScript() const {
+  const QString name(CurrentScriptName());
+  if (loaded_scripts_.contains(name))
+    return loaded_scripts_[name];
+  return NULL;
+}
+
+void PythonEngine::SignalConnectedToPython(PythonQtSignalReceiver* receiver,
+                                           int signal_id, PyObject* callable) {
+  PythonScript* script = static_cast<PythonScript*>(CurrentScript());
+  if (script) {
+    script->RegisterSignalConnection(receiver, signal_id, callable);
+  }
+}

src/scripting/python/pythonengine.h

@@ -50,6 +50,9 @@ public:
   Script* CreateScript(const ScriptInfo& info);
   void DestroyScript(Script* script);
 
+  static QString CurrentScriptName();
+  Script* CurrentScript() const;
+
 public slots:
   void HandleLogRecord(int level, const QString& logger_name, int lineno,
                        const QString& message);
@@ -57,6 +60,8 @@ public slots:
 private slots:
   void PythonStdOut(const QString& str);
   void PythonStdErr(const QString& str);
+  void SignalConnectedToPython(PythonQtSignalReceiver* receiver, int signal_id,
+                               PyObject* callable);
 
 private:
   void AddModuleToModel(const QString& name, PythonQtObjectPtr ptr);

src/scripting/python/pythonscript.cpp

@@ -16,6 +16,7 @@
 */
 
 #include <Python.h>
+#include <PythonQtSignalReceiver.h>
 
 #include "pythonengine.h"
 #include "pythonscript.h"
@@ -75,6 +76,14 @@ bool PythonScript::Init() {
 }
 
 bool PythonScript::Unload() {
+  // Disconnect any signal connections that this script made while it was
+  // running.  This is important because those connections will hold references
+  // to bound methods in the script's classes, so the classes won't get deleted.
+  foreach (const SignalConnection& conn, signal_connections_) {
+    qLog(Debug) << "Disconnecting signal" << conn.signal_id_;
+    conn.receiver_->removeSignalHandler(conn.signal_id_, conn.callable_);
+  }
+
   // Remove this module and all its children from sys.modules.  That should be
   // the only place that references it, so this will clean up the modules'
   // dict and all globals.
@@ -103,5 +112,14 @@ bool PythonScript::Unload() {
   }
 
   module_ = PythonQtObjectPtr();
+
+  PyGC_Collect();
+
   return true;
 }
+
+void PythonScript::RegisterSignalConnection(PythonQtSignalReceiver* receiver,
+                                            int signal_id, PyObject* callable) {
+  qLog(Debug) << "Signal" << signal_id << "registered to an object in" << info().id();
+  signal_connections_ << SignalConnection(receiver, signal_id, callable);
+}

src/scripting/python/pythonscript.h

@@ -33,11 +33,25 @@ public:
   bool Init();
   bool Unload();
 
+  void RegisterSignalConnection(PythonQtSignalReceiver* receiver,
+                                int signal_id, PyObject* callable);
+
 private:
   PythonEngine* engine_;
 
   QString module_name_;
   PythonQtObjectPtr module_;
+
+  struct SignalConnection {
+    SignalConnection(PythonQtSignalReceiver* receiver,
+                     int signal_id, PyObject* callable)
+      : receiver_(receiver), signal_id_(signal_id), callable_(callable) {}
+
+    PythonQtSignalReceiver* receiver_;
+    int signal_id_;
+    PyObject* callable_;
+  };
+  QList<SignalConnection> signal_connections_;
 };
 
 #endif // PYTHONSCRIPT_H

src/scripting/script.cpp

@@ -27,13 +27,3 @@ Script::Script(LanguageEngine* language, const ScriptInfo& info)
 
 Script::~Script() {
 }
-
-void Script::AddNativeObject(QObject* object) {
-  if(!native_objects_.contains(object)) {
-    native_objects_ << object;
-  }
-}
-
-void Script::RemoveNativeObject(QObject* object) {
-  native_objects_.removeAll(object);
-}

src/scripting/script.h

@@ -40,17 +40,9 @@ public:
   const ScriptInfo& info() const { return info_; }
   ScriptInterface* interface() const { return interface_.get(); }
 
-  // The script can "own" QObjects like QActions that must be deleted (and
-  // removed from the UI, etc.) when the script is unloaded.
-  void AddNativeObject(QObject* object);
-  void RemoveNativeObject(QObject* object);
-
   virtual bool Init() = 0;
   virtual bool Unload() = 0;
 
-protected:
-  QList<QObject*> native_objects_;
-
 private:
   Q_DISABLE_COPY(Script);
 

tests/python_test.cpp

@@ -132,6 +132,25 @@ TEST_F(PythonTest, CleanupModuleDict) {
   ASSERT_TRUE(sManager->log_lines_plain().last().endsWith("destructor"));
 }
 
+TEST_F(PythonTest, CleanupSignalConnections) {
+  TemporaryScript script(
+        "from PythonQt.QtCore import QCoreApplication\n"
+        "class Foo:\n"
+        "  def __init__(self):\n"
+        "    QCoreApplication.instance().connect('aboutToQuit()', self.aslot)\n"
+        "  def __del__(self):\n"
+        "    print 'destructor'\n"
+        "  def aslot(self):\n"
+        "    pass\n"
+        "f = Foo()\n");
+  ScriptInfo info;
+  info.InitFromDirectory(sManager, script.directory_);
+
+  sEngine->DestroyScript(sEngine->CreateScript(info));
+
+  ASSERT_TRUE(sManager->log_lines_plain().last().endsWith("destructor"));
+}
+
 TEST_F(PythonTest, ModuleConstants) {
   TemporaryScript script(
         "print type(__builtins__)\n"