From 033918ff790a2e411e9074f4a6c938c4d9b3d7e0 Mon Sep 17 00:00:00 2001 From: David Sansome Date: Mon, 30 May 2011 14:53:59 +0000 Subject: [PATCH] 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 | 5 +++ .../pythonqt/src/PythonQtSignalReceiver.cpp | 22 +++++---- .../pythonqt/src/PythonQtSignalReceiver.h | 1 + src/scripting/python/pythonengine.cpp | 45 +++++++++++++++++++ src/scripting/python/pythonengine.h | 5 +++ src/scripting/python/pythonscript.cpp | 18 ++++++++ src/scripting/python/pythonscript.h | 14 ++++++ src/scripting/script.cpp | 10 ----- src/scripting/script.h | 8 ---- tests/python_test.cpp | 19 ++++++++ 10 files changed, 120 insertions(+), 27 deletions(-) diff --git a/3rdparty/pythonqt/src/PythonQt.h b/3rdparty/pythonqt/src/PythonQt.h index 050dadc50..9a7c29fee 100644 --- a/3rdparty/pythonqt/src/PythonQt.h +++ b/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 diff --git a/3rdparty/pythonqt/src/PythonQtSignalReceiver.cpp b/3rdparty/pythonqt/src/PythonQtSignalReceiver.cpp index d4074b1e4..7bb9dfcac 100644 --- a/3rdparty/pythonqt/src/PythonQtSignalReceiver.cpp +++ b/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 i(_targets); - while (i.hasNext()) { - if (i.next().isSame(sigId, callable)) { - i.remove(); - found = true; - break; - } + QMutableListIterator i(_targets); + while (i.hasNext()) { + if (i.next().isSame(sigId, callable)) { + i.remove(); + found = true; + break; } } return found; diff --git a/3rdparty/pythonqt/src/PythonQtSignalReceiver.h b/3rdparty/pythonqt/src/PythonQtSignalReceiver.h index dfbcbc6ea..028b8536e 100644 --- a/3rdparty/pythonqt/src/PythonQtSignalReceiver.h +++ b/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(); diff --git a/src/scripting/python/pythonengine.cpp b/src/scripting/python/pythonengine.cpp index a3432ff0f..391a8d0ed 100644 --- a/src/scripting/python/pythonengine.cpp +++ b/src/scripting/python/pythonengine.cpp @@ -16,6 +16,8 @@ */ #include +#include + #include #include #include @@ -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(CurrentScript()); + if (script) { + script->RegisterSignalConnection(receiver, signal_id, callable); + } +} diff --git a/src/scripting/python/pythonengine.h b/src/scripting/python/pythonengine.h index 036f92269..38c2c9a36 100644 --- a/src/scripting/python/pythonengine.h +++ b/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); diff --git a/src/scripting/python/pythonscript.cpp b/src/scripting/python/pythonscript.cpp index 053ccea17..65489c649 100644 --- a/src/scripting/python/pythonscript.cpp +++ b/src/scripting/python/pythonscript.cpp @@ -16,6 +16,7 @@ */ #include +#include #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); +} diff --git a/src/scripting/python/pythonscript.h b/src/scripting/python/pythonscript.h index 78c872be6..07b86d8f7 100644 --- a/src/scripting/python/pythonscript.h +++ b/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 signal_connections_; }; #endif // PYTHONSCRIPT_H diff --git a/src/scripting/script.cpp b/src/scripting/script.cpp index 300794661..47288850f 100644 --- a/src/scripting/script.cpp +++ b/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); -} diff --git a/src/scripting/script.h b/src/scripting/script.h index ef5519c01..45e28bbbf 100644 --- a/src/scripting/script.h +++ b/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 native_objects_; - private: Q_DISABLE_COPY(Script); diff --git a/tests/python_test.cpp b/tests/python_test.cpp index 5d1ec604a..0a1c451a1 100644 --- a/tests/python_test.cpp +++ b/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"