Skip to content

Commit

Permalink
Bugfix: do not release Python GIL before script did clean-up
Browse files Browse the repository at this point in the history
  • Loading branch information
joern274 committed Oct 6, 2023
1 parent 38c4b9b commit 1f14f06
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
20 changes: 20 additions & 0 deletions plugins/gui/include/gui/python/python_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,26 @@ namespace hal {
class Module;
class Gate;

/**
* Class to aquire and release GIL
*
* It is important that the GIL is not released before
* Python code has removed all objects created during
* execution. Therefore the GIL release is done at the
* very end when running out of scope and calling the
* destructor
*/
class PythonMutex
{
int mState;
public:
/// Aquire GIL
PythonMutex();

/// Release GIL
~PythonMutex();
};

class PythonThread : public QThread, public PythonContextSubscriber
{
public:
Expand Down
18 changes: 14 additions & 4 deletions plugins/gui/src/python/python_thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@
#include <QDebug>

namespace hal {
PythonMutex::PythonMutex()
{
mState = PyGILState_Ensure();
}

PythonMutex::~PythonMutex()
{
PyGILState_Release((PyGILState_STATE)mState);
}

PythonThread::PythonThread(const QString& script, bool singleStatement, QObject* parent)
: QThread(parent), mScript(script), mSingleStatement(singleStatement),
mAbortRequested(false), mSpamCount(0)
Expand All @@ -26,7 +36,7 @@ namespace hal {
void PythonThread::run()
{
// decides it's our turn.
PyGILState_STATE state = PyGILState_Ensure();
PythonMutex pmutex;
py::dict tmp_context(py::globals());
PythonContext::initializeScript(&tmp_context);

Expand Down Expand Up @@ -60,7 +70,7 @@ namespace hal {
mErrorMessage = QString::fromStdString(std::string(e.what()) + "#\n");
}

PyGILState_Release(state);
// running out of scope calls PyGILState_Release(state);
}

void PythonThread::handleStdout(const QString& output)
Expand Down Expand Up @@ -114,7 +124,7 @@ namespace hal {
mInputMutex.unlock();
}
qDebug() << "about to terminate thread..." << mPythonThreadID;
PyGILState_STATE state = PyGILState_Ensure();
PythonMutex pmutex;
// We interrupt the thread by forcibly injecting an exception
// (namely the KeyboardInterrupt exception, but any other one works as well)
int nThreads = PyThreadState_SetAsyncExc(mPythonThreadID, PyExc_KeyboardInterrupt);
Expand All @@ -127,8 +137,8 @@ namespace hal {
// apparently this can actually happen if you mess up the C<->Python bindings
qDebug() << "Oh no! There seem to be multiple threads with the same ID!";
}
PyGILState_Release(state);
qDebug() << "thread terminated";
// running out of scope calls PyGILState_Release(state);
}

bool PythonThread::getInput(InputType type, QString prompt, QVariant defaultValue)
Expand Down

0 comments on commit 1f14f06

Please sign in to comment.