Fixes empty tracebacks for user Python code

Moving python traceback collection to within runStringUnsafe() so other threads don't clear the global error status before we have a chance to display it to users. fix #34370, #31235
qgis · Feb 10, 2020 · b4c359a · b4c359a
1 parent 58f1206
commit b4c359a
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 18 deletions.
diff --git a/src/python/qgspythonutils.h b/src/python/qgspythonutils.h
@@ -92,10 +92,10 @@ class PYTHON_EXPORT QgsPythonUtils
     virtual bool runString( const QString &command, QString msgOnError = QString(), bool single = true ) = 0;
 
     /**
-     * Runs a Python \a command. No error reporting is not performed.
-     * \returns TRUE if no error occurred
+     * Runs a Python \a command.
+     * \returns empty QString if no error occurred, or Python traceback as a QString on error.
      */
-    virtual bool runStringUnsafe( const QString &command, bool single = true ) = 0;
+    virtual QString runStringUnsafe( const QString &command, bool single = true ) = 0;
 
     /**
      * Evaluates a Python \a command and stores the result in a the \a result string.

diff --git a/src/python/qgspythonutilsimpl.cpp b/src/python/qgspythonutilsimpl.cpp
@@ -294,30 +294,38 @@ void QgsPythonUtilsImpl::uninstallErrorHook()
   runString( QStringLiteral( "qgis.utils.uninstallErrorHook()" ) );
 }
 
-bool QgsPythonUtilsImpl::runStringUnsafe( const QString &command, bool single )
+QString QgsPythonUtilsImpl::runStringUnsafe( const QString &command, bool single )
 {
   // acquire global interpreter lock to ensure we are in a consistent state
   PyGILState_STATE gstate;
   gstate = PyGILState_Ensure();
+  QString ret;
 
   // TODO: convert special characters from unicode strings u"…" to \uXXXX
   // so that they're not mangled to utf-8
   // (non-unicode strings can be mangled)
   PyObject *obj = PyRun_String( command.toUtf8().constData(), single ? Py_single_input : Py_file_input, mMainDict, mMainDict );
-  bool res = nullptr == PyErr_Occurred();
+  PyObject *errobj = PyErr_Occurred();
+  if ( nullptr != errobj )
+  {
+    ret = getTraceback();
+  }
   Py_XDECREF( obj );
 
   // we are done calling python API, release global interpreter lock
   PyGILState_Release( gstate );
 
-  return res;
+  return ret;
 }
 
 bool QgsPythonUtilsImpl::runString( const QString &command, QString msgOnError, bool single )
 {
-  bool res = runStringUnsafe( command, single );
-  if ( res )
+  bool res = true;
+  QString traceback = runStringUnsafe( command, single );
+  if ( traceback.isEmpty() )
     return true;
+  else
+    res = false;
 
   if ( msgOnError.isEmpty() )
   {
@@ -327,7 +335,6 @@ bool QgsPythonUtilsImpl::runString( const QString &command, QString msgOnError,
 
   // TODO: use python implementation
 
-  QString traceback = getTraceback();
   QString path, version;
   evalString( QStringLiteral( "str(sys.path)" ), path );
   evalString( QStringLiteral( "sys.version" ), version );
@@ -352,10 +359,6 @@ QString QgsPythonUtilsImpl::getTraceback()
 {
 #define TRACEBACK_FETCH_ERROR(what) {errMsg = what; goto done;}
 
-  // acquire global interpreter lock to ensure we are in a consistent state
-  PyGILState_STATE gstate;
-  gstate = PyGILState_Ensure();
-
   QString errMsg;
   QString result;
 
@@ -364,7 +367,7 @@ QString QgsPythonUtilsImpl::getTraceback()
   PyObject *obStringIO = nullptr;
   PyObject *obResult = nullptr;
 
-  PyObject *type, *value, *traceback;
+  PyObject *type = nullptr, *value = nullptr, *traceback = nullptr;
 
   PyErr_Fetch( &type, &value, &traceback );
   PyErr_NormalizeException( &type, &value, &traceback );
@@ -423,9 +426,6 @@ QString QgsPythonUtilsImpl::getTraceback()
   Py_XDECREF( traceback );
   Py_XDECREF( type );
 
-  // we are done calling python API, release global interpreter lock
-  PyGILState_Release( gstate );
-
   return result;
 }
 

diff --git a/src/python/qgspythonutilsimpl.h b/src/python/qgspythonutilsimpl.h
@@ -44,7 +44,7 @@ class QgsPythonUtilsImpl : public QgsPythonUtils
     void exitPython() override;
     bool isEnabled() override;
     bool runString( const QString &command, QString msgOnError = QString(), bool single = true ) override;
-    bool runStringUnsafe( const QString &command, bool single = true ) override;
+    QString runStringUnsafe( const QString &command, bool single = true ) override; // returns error traceback on failure, empty QString on success
     bool evalString( const QString &command, QString &result ) override;
     bool getError( QString &errorClassName, QString &errorText ) override;