From 26335d6909761cec0ca748257faacc282bf770aa Mon Sep 17 00:00:00 2001 From: Martyn Gigg Date: Sun, 3 Sep 2017 21:18:58 +0100 Subject: [PATCH 1/3] Alter ScopedPythonGIL to follow the std::lock_guard pattern This allows a PythonGIL object to be stored in order to workaround a quirk in the Python GILStateAPI --- MantidPlot/src/Mantid/MantidUI.cpp | 6 +- MantidPlot/src/PythonScript.cpp | 72 ++++++++++--------- MantidPlot/src/PythonScript.h | 39 ++++++++-- MantidPlot/src/PythonScripting.cpp | 5 +- MantidPlot/src/PythonScripting.h | 4 ++ .../MantidQtWidgets/Common/PythonThreading.h | 7 +- qt/widgets/common/src/PythonThreading.cpp | 14 +++- 7 files changed, 97 insertions(+), 50 deletions(-) diff --git a/MantidPlot/src/Mantid/MantidUI.cpp b/MantidPlot/src/Mantid/MantidUI.cpp index faed1265c11f..4d2d027c734e 100644 --- a/MantidPlot/src/Mantid/MantidUI.cpp +++ b/MantidPlot/src/Mantid/MantidUI.cpp @@ -420,7 +420,8 @@ void MantidUI::shutdown() { // If any python objects need to be cleared away then the GIL needs to be // held. - ScopedPythonGIL gil; + PythonGIL gil; + ScopedPythonGIL lock(gil); // Relevant notifications are connected to signals that will close all // dependent windows Mantid::API::FrameworkManager::Instance().shutdown(); @@ -2216,7 +2217,8 @@ void MantidUI::clearAllMemory(const bool prompt) { // If any python objects need to be cleared away then the GIL needs to be // held. This doesn't feel like // it is in the right place but it will do no harm - ScopedPythonGIL gil; + PythonGIL gil; + ScopedPythonGIL lock(gil); // Relevant notifications are connected to signals that will close all // dependent windows Mantid::API::FrameworkManager::Instance().clear(); diff --git a/MantidPlot/src/PythonScript.cpp b/MantidPlot/src/PythonScript.cpp index 81400c8e9803..78438e29ed58 100644 --- a/MantidPlot/src/PythonScript.cpp +++ b/MantidPlot/src/PythonScript.cpp @@ -78,10 +78,10 @@ static const QString MSG_STARTED = "Script execution started."; */ PythonScript::PythonScript(PythonScripting *env, const QString &name, const InteractionType interact, QObject *context) - : Script(env, name, interact, context), m_pythonEnv(env), localDict(NULL), + : Script(env, name, interact, context), m_interp(env), localDict(NULL), stdoutSave(NULL), stderrSave(NULL), m_codeFileObject(NULL), m_threadID(-1), isFunction(false), m_isInitialized(false), - m_pathHolder(name), m_workspaceHandles() { + m_pathHolder(name, *this), m_workspaceHandles() { initialize(name, context); } @@ -89,7 +89,7 @@ PythonScript::PythonScript(PythonScripting *env, const QString &name, * Destructor */ PythonScript::~PythonScript() { - ScopedPythonGIL lock; + ScopedPythonGIL lock(gil()); this->abort(); observeAdd(false); observeAfterReplace(false); @@ -142,7 +142,7 @@ PyObject *PythonScript::createSipInstanceFromMe() { */ bool PythonScript::compilesToCompleteStatement(const QString &code) const { bool result(false); - ScopedPythonGIL gil; + ScopedPythonGIL lock(gil()); PyObject *compiledCode = Py_CompileString(code.toAscii(), "", Py_file_input); if (PyObject *exception = PyErr_Occurred()) { // Certain exceptions still mean the code is complete @@ -186,16 +186,16 @@ void PythonScript::sendLineChangeSignal(int lineNo, bool error) { * Create a list autocomplete keywords */ void PythonScript::generateAutoCompleteList() { - ScopedPythonGIL gil; + ScopedPythonGIL lock(gil()); PyObject *keywords = PyObject_CallFunctionObjArgs( - PyDict_GetItemString(m_pythonEnv->globalDict(), + PyDict_GetItemString(m_interp->globalDict(), "_ScopeInspector_GetFunctionAttributes"), localDict, NULL); if (PyErr_Occurred() || !keywords) { PyErr_Print(); return; } - QStringList keywordList = pythonEnv()->toStringList(keywords); + QStringList keywordList = interp()->toStringList(keywords); Py_DECREF(keywords); emit autoCompleteListGenerated(keywordList); } @@ -206,7 +206,7 @@ void PythonScript::generateAutoCompleteList() { */ void PythonScript::emit_error() { // gil is necessary so other things don't continue - ScopedPythonGIL gil; + ScopedPythonGIL lock(gil()); // return early if nothing happened if (!PyErr_Occurred()) { @@ -286,12 +286,12 @@ void PythonScript::emit_error() { * @param syntaxError An instance of PyExc_SyntaxError */ QString PythonScript::constructSyntaxErrorStr(PyObject *syntaxError) { - QString exceptionAsStr = m_pythonEnv->toString(syntaxError); + QString exceptionAsStr = m_interp->toString(syntaxError); exceptionAsStr = exceptionAsStr.section("(", 0, 0).trimmed(); - const QString filename = m_pythonEnv->toString( - PyObject_GetAttrString(syntaxError, "filename"), true); + const QString filename = + m_interp->toString(PyObject_GetAttrString(syntaxError, "filename"), true); int lineno = static_cast( - m_pythonEnv->toLong(PyObject_GetAttrString(syntaxError, "lineno"))); + m_interp->toLong(PyObject_GetAttrString(syntaxError, "lineno"))); #if PY_VERSION_HEX < 0x02070000 // Syntax errors generated by earlier versions seem to have a line number off // by 1 @@ -303,9 +303,9 @@ QString PythonScript::constructSyntaxErrorStr(PyObject *syntaxError) { // using a ^ character PyObject *textObject = PyObject_GetAttrString(syntaxError, "text"); if (textObject != Py_None) { - QString text = m_pythonEnv->toString(textObject, true).trimmed(); + QString text = m_interp->toString(textObject, true).trimmed(); int offset = static_cast( - m_pythonEnv->toLong(PyObject_GetAttrString(syntaxError, "offset"))); + m_interp->toLong(PyObject_GetAttrString(syntaxError, "offset"))); QString offsetMarker = QString(offset - 1, ' ') + "^"; msg = "File \"%1\", line %2\n" " %3\n" @@ -357,17 +357,17 @@ void PythonScript::tracebackToMsg(QTextStream &msgStream, bool PythonScript::setQObject(QObject *val, const char *name) { if (localDict) { - return pythonEnv()->setQObject(val, name, localDict); + return interp()->setQObject(val, name, localDict); } else return false; } bool PythonScript::setInt(int val, const char *name) { - return pythonEnv()->setInt(val, name, localDict); + return interp()->setInt(val, name, localDict); } bool PythonScript::setDouble(double val, const char *name) { - return pythonEnv()->setDouble(val, name, localDict); + return interp()->setDouble(val, name, localDict); } void PythonScript::setContext(QObject *context) { @@ -380,7 +380,7 @@ void PythonScript::setContext(QObject *context) { * the dictionary context back to the default set */ void PythonScript::clearLocals() { - ScopedPythonGIL pythonLock; + ScopedPythonGIL lock(gil()); PyObject *mainModule = PyImport_AddModule("__main__"); PyObject *cleanLocals = PyDict_Copy(PyModule_GetDict(mainModule)); @@ -397,6 +397,12 @@ void PythonScript::clearLocals() { localDict = cleanLocals; } +/** + * @brief PythonScript::gil + * @return A reference to the global lock + */ +PythonGIL &PythonScript::gil() const { return interp()->gil(); } + /** * Sets the context for the script and if name points to a file then * sets the __file__ variable @@ -406,7 +412,7 @@ void PythonScript::clearLocals() { void PythonScript::initialize(const QString &name, QObject *context) { clearLocals(); // holds and releases GIL - ScopedPythonGIL pythonlock; + ScopedPythonGIL lock(gil()); PythonScript::setIdentifier(name); setContext(context); @@ -426,12 +432,12 @@ void PythonScript::initialize(const QString &name, QObject *context) { void PythonScript::beginStdoutRedirect() { if (!redirectStdOut()) return; - stdoutSave = PyDict_GetItemString(pythonEnv()->sysDict(), "stdout"); + stdoutSave = PyDict_GetItemString(interp()->sysDict(), "stdout"); Py_XINCREF(stdoutSave); - stderrSave = PyDict_GetItemString(pythonEnv()->sysDict(), "stderr"); + stderrSave = PyDict_GetItemString(interp()->sysDict(), "stderr"); Py_XINCREF(stderrSave); - pythonEnv()->setQObject(this, "stdout", pythonEnv()->sysDict()); - pythonEnv()->setQObject(this, "stderr", pythonEnv()->sysDict()); + interp()->setQObject(this, "stdout", interp()->sysDict()); + interp()->setQObject(this, "stderr", interp()->sysDict()); } /** @@ -442,9 +448,9 @@ void PythonScript::endStdoutRedirect() { if (!redirectStdOut()) return; - PyDict_SetItemString(pythonEnv()->sysDict(), "stdout", stdoutSave); + PyDict_SetItemString(interp()->sysDict(), "stdout", stdoutSave); Py_XDECREF(stdoutSave); - PyDict_SetItemString(pythonEnv()->sysDict(), "stderr", stderrSave); + PyDict_SetItemString(interp()->sysDict(), "stderr", stderrSave); Py_XDECREF(stderrSave); } @@ -464,7 +470,7 @@ bool PythonScript::compileImpl() { * @return */ QVariant PythonScript::evaluateImpl() { - ScopedPythonGIL gil; + ScopedPythonGIL lock(gil()); PyObject *compiledCode = this->compileToByteCode(true); if (!compiledCode) { return QVariant(""); @@ -590,8 +596,8 @@ void PythonScript::abortImpl() { // hasn't implemented cancel() checking so that when control returns the // Python the // interrupt should be picked up. - ScopedPythonGIL lock; - m_pythonEnv->raiseAsyncException(m_threadID, PyExc_KeyboardInterrupt); + ScopedPythonGIL lock(gil()); + m_interp->raiseAsyncException(m_threadID, PyExc_KeyboardInterrupt); PyObject *curAlg = PyObject_CallFunction(m_algorithmInThread, STR_LITERAL("l"), m_threadID); if (curAlg && curAlg != Py_None) { @@ -605,7 +611,7 @@ void PythonScript::abortImpl() { * @return A long int giving a unique ID for the thread */ long PythonScript::getThreadID() { - ScopedPythonGIL lock; + ScopedPythonGIL lock(gil()); return PyThreadState_Get()->thread_id; } @@ -613,7 +619,7 @@ long PythonScript::getThreadID() { bool PythonScript::executeString() { emit started(MSG_STARTED); bool success(false); - ScopedPythonGIL gil; + ScopedPythonGIL lock(gil()); PyObject *compiledCode = compileToByteCode(false); PyObject *result(NULL); @@ -629,7 +635,7 @@ bool PythonScript::executeString() { // call Algorithm::cancel to make sure we capture it. The doubling // can leave an interrupt in the pipeline so we clear it was we've // got the error info out - m_pythonEnv->raiseAsyncException(m_threadID, NULL); + m_interp->raiseAsyncException(m_threadID, NULL); } else { emit finished(MSG_FINISHED); success = true; @@ -706,7 +712,7 @@ PyObject *PythonScript::compileToByteCode(bool for_eval) { // __builtins__. PyDict_SetItemString( localDict, "__builtins__", - PyDict_GetItemString(pythonEnv()->globalDict(), "__builtins__")); + PyDict_GetItemString(interp()->globalDict(), "__builtins__")); PyObject *ret = PyRun_String( "def col(c,*arg):\n" "\ttry: return self.cell(c,arg[0])\n" @@ -728,7 +734,7 @@ PyObject *PythonScript::compileToByteCode(bool for_eval) { // __builtins__. PyDict_SetItemString( localDict, "__builtins__", - PyDict_GetItemString(pythonEnv()->globalDict(), "__builtins__")); + PyDict_GetItemString(interp()->globalDict(), "__builtins__")); PyObject *ret = PyRun_String("def cell(*arg):\n" "\ttry: return self.cell(arg[0],arg[1])\n" diff --git a/MantidPlot/src/PythonScript.h b/MantidPlot/src/PythonScript.h index 0835fd8cdc9f..4372ad93f14e 100644 --- a/MantidPlot/src/PythonScript.h +++ b/MantidPlot/src/PythonScript.h @@ -105,7 +105,8 @@ class PythonScript : public Script, MantidQt::API::WorkspaceObserver { /// Helper class to ensure the sys.path variable is updated correctly struct PythonPathHolder { /// Update the path with the given entry - explicit PythonPathHolder(const QString &entry) : m_path(entry) { + explicit PythonPathHolder(const QString &entry, const PythonScript &script) + : m_path(entry), m_script(script) { const QFileInfo filePath(m_path); if (filePath.exists()) { QDir directory = filePath.absoluteDir(); @@ -125,14 +126,14 @@ class PythonScript : public Script, MantidQt::API::WorkspaceObserver { } void appendPath(const QString &path) { - ScopedPythonGIL pythonLock; + ScopedPythonGIL lock(m_script.gil()); QString code = "if r'%1' not in sys.path:\n" " sys.path.append(r'%1')"; code = code.arg(path); PyRun_SimpleString(code.toAscii().constData()); } void removePath(const QString &path) { - ScopedPythonGIL pythonLock; + ScopedPythonGIL lock(m_script.gil()); QString code = "if r'%1' in sys.path:\n" " sys.path.remove(r'%1')"; code = code.arg(path); @@ -141,15 +142,39 @@ class PythonScript : public Script, MantidQt::API::WorkspaceObserver { private: QString m_path; + const PythonScript &m_script; }; - inline PythonScripting *pythonEnv() const { return m_pythonEnv; } + inline PythonScripting *interp() const { return m_interp; } + PythonGIL &gil() const; void initialize(const QString &name, QObject *context); void beginStdoutRedirect(); void endStdoutRedirect(); - // --------------------------- Script compilation/execution - // ----------------------------------- + // Pre and post-processing options for recursive calls. + // The Python GILState api is supposed to be able to ensure + // that the current thread is ready to execute Python code + // regardless of the current interpreter thread state. + // However, we have a case with the ApplicationWindow::runPythonScript + // method where a script can be sent to that method to be run with + // async=false yet the script itself contains a runPythonScript(async=True) + // call. This causes recursion into the PythonGIL object where + // the initial GIL has not been released and a deadlock ensues. + // An example script sent to runPythonScript(async=False) would be + // + // import mantidplot + // for i in range(10): + // mantidplot.runPythonScript('test=CreateSampleWorkspace()', True) + // + // To circumvent this we must release the GIL on the main thread + // before starting the async thread and then reaquire it when that thread + // has finished and the main thread must keep executing. These methods + // are used for this purpose and are NOT used by the general executeAsync + // methods where the GILState API functions can cope and there is no + // recursion. + virtual void recursiveAsyncSetup() override; + virtual void recursiveAsyncTeardown() override; + /// Compile the code, returning true if it was successful, false otherwise bool compileImpl() override; /// Evaluate the current code and return a result as a QVariant @@ -191,7 +216,7 @@ class PythonScript : public Script, MantidQt::API::WorkspaceObserver { /// Send out an error and clear it from python. void emit_error(); - PythonScripting *m_pythonEnv; + PythonScripting *m_interp; PyObject *localDict, *stdoutSave, *stderrSave; PyObject *m_codeFileObject; long m_threadID; ///< Python thread id diff --git a/MantidPlot/src/PythonScripting.cpp b/MantidPlot/src/PythonScripting.cpp index bbe351ed30e5..d6ea4bb74be6 100644 --- a/MantidPlot/src/PythonScripting.cpp +++ b/MantidPlot/src/PythonScripting.cpp @@ -69,7 +69,7 @@ ScriptingEnv *PythonScripting::constructor(ApplicationWindow *parent) { /** Constructor */ PythonScripting::PythonScripting(ApplicationWindow *parent) : ScriptingEnv(parent, "Python"), m_globals(NULL), m_math(NULL), - m_sys(NULL), m_mainThreadState(NULL) {} + m_sys(NULL), m_mainThreadState(NULL), m_gil() {} PythonScripting::~PythonScripting() {} @@ -77,8 +77,7 @@ PythonScripting::~PythonScripting() {} * @param args A list of strings that denoting command line arguments */ void PythonScripting::setSysArgs(const QStringList &args) { - ScopedPythonGIL gil; - + ScopedPythonGIL lock(gil()); PyObject *argv = toPyList(args); if (argv && m_sys) { PyDict_SetItemString(m_sys, "argv", argv); diff --git a/MantidPlot/src/PythonScripting.h b/MantidPlot/src/PythonScripting.h index e9d4aaf8ead2..9180d1aa87c1 100644 --- a/MantidPlot/src/PythonScripting.h +++ b/MantidPlot/src/PythonScripting.h @@ -113,6 +113,8 @@ class PythonScripting : public ScriptingEnv { PyObject *globalDict() { return m_globals; } /// Return the sys dictionary for this environment PyObject *sysDict() { return m_sys; } + /// Return a reference to the global lock + PythonGIL &gil() { return m_gil; } private: /// Constructor @@ -139,6 +141,8 @@ class PythonScripting : public ScriptingEnv { PyObject *m_sys; /// Pointer to the main threads state PyThreadState *m_mainThreadState; + /// Wrap's acquisition of the GIL + PythonGIL m_gil; }; #endif diff --git a/qt/widgets/common/inc/MantidQtWidgets/Common/PythonThreading.h b/qt/widgets/common/inc/MantidQtWidgets/Common/PythonThreading.h index 5d2a7e167c67..2b33468c29e1 100644 --- a/qt/widgets/common/inc/MantidQtWidgets/Common/PythonThreading.h +++ b/qt/widgets/common/inc/MantidQtWidgets/Common/PythonThreading.h @@ -16,6 +16,7 @@ class EXPORT_OPT_MANTIDQT_COMMON PythonGIL { public: PythonGIL(); + inline bool locked() const { return m_acquired; } void acquire(); void release(); @@ -23,6 +24,7 @@ class EXPORT_OPT_MANTIDQT_COMMON PythonGIL { PythonGIL(const PythonGIL &); /// Current GIL state PyGILState_STATE m_state; + bool m_acquired; }; //------------------------------------------------------------------------------ @@ -51,15 +53,16 @@ class EXPORT_OPT_MANTIDQT_COMMON RecursivePythonGIL { /** * Acquires a lock in the constructor and releases it in the destructor. + * Modelled on std::lock_guard * @tparam T Templated on the lock type */ template class ScopedGIL { public: - ScopedGIL() : m_lock() { m_lock.acquire(); } + ScopedGIL(T &l) : m_lock(l) { m_lock.acquire(); } ~ScopedGIL() { m_lock.release(); } private: - T m_lock; + T &m_lock; }; /// Typedef for scoped lock diff --git a/qt/widgets/common/src/PythonThreading.cpp b/qt/widgets/common/src/PythonThreading.cpp index 973b7aa1c557..cbe5dd54c542 100644 --- a/qt/widgets/common/src/PythonThreading.cpp +++ b/qt/widgets/common/src/PythonThreading.cpp @@ -4,6 +4,8 @@ #include "MantidQtWidgets/Common/PythonThreading.h" #include +#include + //------------------------------------------------------------------------------ // PythonGIL Public members //------------------------------------------------------------------------------ @@ -12,18 +14,24 @@ * Leaves the lock unlocked. You are strongly encouraged to use * the ScopedInterpreterLock class to control this */ -PythonGIL::PythonGIL() : m_state(PyGILState_UNLOCKED) {} +PythonGIL::PythonGIL() : m_state(PyGILState_UNLOCKED), m_acquired(false) {} /** * Calls PyGILState_Ensure. A call to this must be matched by a call to release * on the same thread. */ -void PythonGIL::acquire() { m_state = PyGILState_Ensure(); } +void PythonGIL::acquire() { + m_state = PyGILState_Ensure(); + m_acquired = true; +} /** * Calls PyGILState_Release */ -void PythonGIL::release() { PyGILState_Release(m_state); } +void PythonGIL::release() { + PyGILState_Release(m_state); + m_acquired = false; +} //------------------------------------------------------------------------------ // RecursivePythonGIL public members From 4036cd37b623600c5f3b51791caff9e4c0850111 Mon Sep 17 00:00:00 2001 From: Martyn Gigg Date: Sun, 3 Sep 2017 21:20:15 +0100 Subject: [PATCH 2/3] Add pre/post setup for recursive async calls --- MantidPlot/src/ApplicationWindow.cpp | 25 ++++-- MantidPlot/src/PythonScript.cpp | 128 +++++---------------------- MantidPlot/src/PythonScript.h | 18 ---- MantidPlot/src/Script.cpp | 11 --- MantidPlot/src/Script.h | 11 ++- 5 files changed, 41 insertions(+), 152 deletions(-) diff --git a/MantidPlot/src/ApplicationWindow.cpp b/MantidPlot/src/ApplicationWindow.cpp index 765cc32e6876..20d5eac82f27 100644 --- a/MantidPlot/src/ApplicationWindow.cpp +++ b/MantidPlot/src/ApplicationWindow.cpp @@ -573,8 +573,8 @@ void ApplicationWindow::init(bool factorySettings, const QStringList &args) { // Scripting m_script_envs = QHash(); + m_iface_script = nullptr; setScriptingLanguage(defaultScriptingLang); - m_iface_script = NULL; m_interpreterDock = new QDockWidget(this); m_interpreterDock->setObjectName( @@ -15043,10 +15043,9 @@ bool ApplicationWindow::runPythonScript(const QString &code, bool async, bool quiet, bool redirect) { if (code.isEmpty()) return false; - - if (m_iface_script == NULL) { + if (!m_iface_script) { if (setScriptingLanguage("Python")) { - m_iface_script = scriptingEnv()->newScript("", NULL, + m_iface_script = scriptingEnv()->newScript("", nullptr, Script::NonInteractive); } else { return false; @@ -15065,12 +15064,20 @@ bool ApplicationWindow::runPythonScript(const QString &code, bool async, } bool success(false); if (async) { - QFuture job = m_iface_script->executeAsync(ScriptCode(code)); - while (job.isRunning()) { - QCoreApplication::instance()->processEvents(); + m_iface_script->recursiveAsyncSetup(); + auto job = m_iface_script->executeAsync(ScriptCode(code)); + // Start a local event loop to keep processing events + // while we are running the script. Inspired by the IPython + // Qt inputhook in IPython.terminal.pt_inputhooks.qt + QEventLoop eventLoop(QApplication::instance()); + QTimer timer; + connect(&timer, SIGNAL(timeout()), &eventLoop, SLOT(quit())); + while (!job.isFinished()) { + timer.start(50); + eventLoop.exec(); + timer.stop(); } - // Ensure the remaining events are processed - QCoreApplication::instance()->processEvents(); + m_iface_script->recursiveAsyncTeardown(); success = job.result(); } else { success = m_iface_script->execute(ScriptCode(code)); diff --git a/MantidPlot/src/PythonScript.cpp b/MantidPlot/src/PythonScript.cpp index 78438e29ed58..c0b617a2e0a0 100644 --- a/MantidPlot/src/PythonScript.cpp +++ b/MantidPlot/src/PythonScript.cpp @@ -454,6 +454,26 @@ void PythonScript::endStdoutRedirect() { Py_XDECREF(stderrSave); } +/** + * To be called from the main thread before an async call that is + * recursive. See ApplicationWindow::runPythonScript + */ +void PythonScript::recursiveAsyncSetup() { + if (gil().locked()) { + gil().release(); + } +} + +/** + * To be called from the main thread immediately after an async call + * that is recursive. See ApplicationWindow::runPythonScript + */ +void PythonScript::recursiveAsyncTeardown() { + if (!gil().locked()) { + gil().acquire(); + } +} + /** * Compile the code returning true if successful, false otherwise * @param code @@ -793,111 +813,3 @@ PyObject *PythonScript::compileToByteCode(bool for_eval) { } return compiledCode; } - -//-------------------------------------------------------------------------------------------- - -/** - * Listen to add notifications from the ADS and add a Python variable of the - * workspace name - * to the current scope - * @param wsName The name of the workspace - * @param ws The ws ptr (unused) - */ -void PythonScript::addHandle(const std::string &wsName, - const Mantid::API::Workspace_sptr ws) { - addPythonReference(wsName, ws); -} - -/** - * Listen to add/replace notifications from the ADS and add a Python variable - * of - * the workspace name - * to the current scope - * @param wsName The name of the workspace - * @param ws The ws ptr (unused) - */ -void PythonScript::afterReplaceHandle(const std::string &wsName, - const Mantid::API::Workspace_sptr ws) { - addPythonReference(wsName, ws); -} - -/** - * Removes a Python variable of the workspace name from the current scope - * @param wsName The name of the workspace - * @param ws The ws ptr (unused) - */ -void PythonScript::postDeleteHandle(const std::string &wsName) { - deletePythonReference(wsName); -} - -/** - * Clear all workspace handle references - */ -void PythonScript::clearADSHandle() { - for (auto itr = m_workspaceHandles.cbegin(); - itr != m_workspaceHandles.cend();) { - // This also erases the element from current set. The standard says that - // erase only invalidates - // iterators of erased elements so we need to increment the iterator and - // get - // back the previous value - // i.e. the postfix operator - this->deletePythonReference(*(itr++)); - } - - assert(m_workspaceHandles.empty()); -} - -/** - * Add a Python variable of the workspace name - * to the current scope - * @param wsName The name of the workspace - * @param ws The ws ptr (unused) - */ -void PythonScript::addPythonReference(const std::string &wsName, - const Mantid::API::Workspace_sptr ws) { - UNUSED_ARG(ws); - - // Compile a code object - const size_t length = wsName.length() * 2 + 10; - char *code = new char[length + 1]; - const char *name = wsName.c_str(); - sprintf(code, "%s = mtd['%s']", name, name); - PyObject *codeObj = - Py_CompileString(code, "PythonScript::addPythonReference", Py_file_input); - if (codeObj) { - PyObject *ret = PyEval_EvalCode(CODE_OBJECT(codeObj), localDict, localDict); - Py_XDECREF(ret); - } - if (PyErr_Occurred()) { - PyErr_Clear(); - } else { - // Keep track of it - m_workspaceHandles.insert(m_workspaceHandles.end(), wsName); - } - Py_XDECREF(codeObj); - delete[] code; -} - -/** - * Delete a Python reference to the given workspace name - * @param wsName The name of the workspace - */ -void PythonScript::deletePythonReference(const std::string &wsName) { - const size_t length = wsName.length() + 4; - char *code = new char[length + 1]; - sprintf(code, "del %s", wsName.c_str()); - PyObject *codeObj = - Py_CompileString(code, "PythonScript::deleteHandle", Py_file_input); - if (codeObj) { - PyObject *ret = PyEval_EvalCode(CODE_OBJECT(codeObj), localDict, localDict); - Py_XDECREF(ret); - } - if (PyErr_Occurred()) { - PyErr_Clear(); - } else { - m_workspaceHandles.erase(wsName); - } - Py_XDECREF(codeObj); - delete[] code; -} diff --git a/MantidPlot/src/PythonScript.h b/MantidPlot/src/PythonScript.h index 4372ad93f14e..33d23551d1aa 100644 --- a/MantidPlot/src/PythonScript.h +++ b/MantidPlot/src/PythonScript.h @@ -195,24 +195,6 @@ class PythonScript : public Script, MantidQt::API::WorkspaceObserver { /// Compile to bytecode PyObject *compileToByteCode(bool for_eval = true); - // ---------------------------- Variable reference - // --------------------------------------------- - /// Listen to add notifications from the ADS - void addHandle(const std::string &wsName, - const Mantid::API::Workspace_sptr ws) override; - /// Listen to add/replace notifications from the ADS - void afterReplaceHandle(const std::string &wsName, - const Mantid::API::Workspace_sptr ws) override; - /// Listen to delete notifications - void postDeleteHandle(const std::string &wsName) override; - /// Listen to ADS clear notifications - void clearADSHandle() override; - /// Add/update a Python reference to the given workspace - void addPythonReference(const std::string &wsName, - const Mantid::API::Workspace_sptr ws); - /// Delete a Python reference to the given workspace name - void deletePythonReference(const std::string &wsName); - /// Send out an error and clear it from python. void emit_error(); diff --git a/MantidPlot/src/Script.cpp b/MantidPlot/src/Script.cpp index d82f2d5383b0..ab0019e99a98 100644 --- a/MantidPlot/src/Script.cpp +++ b/MantidPlot/src/Script.cpp @@ -158,14 +158,3 @@ void Script::setIsRunning() { m_execMode = Running; } * Sets the offset & code string */ void Script::setupCode(const ScriptCode &code) { m_code = code; } - -/** - * Ensure that any line endings are converted to single '\n' so that the Python - * C API is happy - * @param text :: The text to check and convert - */ -QString Script::normaliseLineEndings(QString text) const { - text = text.replace(QRegExp("\\r\\n"), QString("\n")); - text = text.replace(QRegExp("\\r"), QString("\n")); - return text; -} diff --git a/MantidPlot/src/Script.h b/MantidPlot/src/Script.h index d6404b3af2ee..0baf7eebeccc 100644 --- a/MantidPlot/src/Script.h +++ b/MantidPlot/src/Script.h @@ -91,6 +91,9 @@ class Script : public QObject { bool redirectStdOut() const { return m_redirectOutput; } void redirectStdOut(bool on) { m_redirectOutput = on; } + virtual void recursiveAsyncSetup() {} + virtual void recursiveAsyncTeardown() {} + /// Create a list of keywords for the code completion API virtual void generateAutoCompleteList() {} // Does the code compile to a complete statement, i.e no more input is @@ -152,6 +155,8 @@ public slots: virtual bool executeImpl() = 0; /// Implementation of the abort request virtual void abortImpl() = 0; + /// Setup the code from a script code object + void setupCode(const ScriptCode &code); private: /** @@ -175,12 +180,6 @@ public slots: ScriptThreadPool(); }; - /// Setup the code from a script code object - void setupCode(const ScriptCode &code); - /// Normalise line endings for the given code. The Python C/API does not seem - /// to like CRLF endings so normalise to just LF - QString normaliseLineEndings(QString text) const; - ScriptingEnv *m_env; std::string m_name; // Easier to convert to C string ScriptCode m_code; From 32ab9c3d1e0334808e5543c55c33ee57ba8dc7ee Mon Sep 17 00:00:00 2001 From: Martyn Gigg Date: Sun, 3 Sep 2017 21:21:09 +0100 Subject: [PATCH 3/3] Remove redundant link library --- MantidPlot/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/MantidPlot/CMakeLists.txt b/MantidPlot/CMakeLists.txt index 1ebc31e3f63e..0fa6c5e8c92a 100644 --- a/MantidPlot/CMakeLists.txt +++ b/MantidPlot/CMakeLists.txt @@ -789,7 +789,6 @@ add_executable ( MantidPlot ${WIN_CONSOLE} MACOSX_BUNDLE ${ALL_SRC} src/main.cpp # Library dependencies target_link_libraries ( MantidPlot LINK_PRIVATE ${TCMALLOC_LIBRARIES_LINKTIME} ${CORE_MANTIDLIBS} - API MantidQtWidgetsCommon MantidQtWidgetsInstrumentView MantidQtWidgetsSliceViewer