From 783e7d47a5e9128e8fff6b7d5eda37d4c25eb717 Mon Sep 17 00:00:00 2001
From: baldurk <baldurk@baldurk.org>
Date: Thu, 20 Feb 2025 16:13:52 +0000
Subject: [PATCH] Make python extension loading more robust to missing
 register/unregister

---
 qrenderdoc/Code/pyrenderdoc/PythonContext.cpp | 84 +++++++++++++------
 1 file changed, 60 insertions(+), 24 deletions(-)

diff --git a/qrenderdoc/Code/pyrenderdoc/PythonContext.cpp b/qrenderdoc/Code/pyrenderdoc/PythonContext.cpp
index 73370cd9e..238f482cd 100644
--- a/qrenderdoc/Code/pyrenderdoc/PythonContext.cpp
+++ b/qrenderdoc/Code/pyrenderdoc/PythonContext.cpp
@@ -142,6 +142,15 @@ static QMutex decrefQueueMutex;
 static QList<PyObject *> decrefQueue;
 extern "C" void ProcessDecRefQueue();
 
+// helper overload to give us the semantics we want - return NULL without an exception if the attr doesn't exist
+PyObject *PyObject_SafeGetAttrString(PyObject *obj, const char *string)
+{
+  if(PyObject_HasAttrString(obj, string) == 0)
+    return NULL;
+
+  return PyObject_GetAttrString(obj, string);
+}
+
 void FetchException(QString &typeStr, QString &valueStr, int &finalLine, QList<QString> &frames)
 {
   PyObject *exObj = NULL, *valueObj = NULL, *tracebackObj = NULL;
@@ -170,7 +179,7 @@ void FetchException(QString &typeStr, QString &valueStr, int &finalLine, QList<Q
 
     if(tracebackModule)
     {
-      PyObject *func = PyObject_GetAttrString(tracebackModule, "format_tb");
+      PyObject *func = PyObject_SafeGetAttrString(tracebackModule, "format_tb");
 
       if(func && PyCallable_Check(func))
       {
@@ -199,6 +208,10 @@ void FetchException(QString &typeStr, QString &valueStr, int &finalLine, QList<Q
 
         Py_DecRef(args);
       }
+      else
+      {
+        qCritical() << "Couldn't get format_tb from traceback module";
+      }
     }
   }
 
@@ -338,7 +351,10 @@ void PythonContext::GlobalInit()
 // if we need to append to sys.path to locate PySide2, do that now
 #if defined(PYSIDE2_SYS_PATH)
   {
-    PyObject *syspath = PyObject_GetAttrString(sysobj, "path");
+    PyObject *syspath = PyObject_SafeGetAttrString(sysobj, "path");
+
+    if(!syspath)
+      qCritical() << "couldn't get sys.path";
 
 #ifndef STRINGIZE
 #define STRINGIZE2(a) #a
@@ -364,7 +380,10 @@ void PythonContext::GlobalInit()
 
     if(QDir(testpath).exists(lit("run_tests.py")))
     {
-      PyObject *syspath = PyObject_GetAttrString(sysobj, "path");
+      PyObject *syspath = PyObject_SafeGetAttrString(sysobj, "path");
+
+      if(!syspath)
+        qCritical() << "couldn't get sys.path";
 
       PyObject *str = PyUnicode_FromString(testpath.toUtf8().data());
 
@@ -447,7 +466,7 @@ PythonContext::PythonContext(QObject *parent) : QObject(parent)
 
   if(rlcompleter)
   {
-    PyObject *Completer = PyObject_GetAttrString(rlcompleter, "Completer");
+    PyObject *Completer = PyObject_SafeGetAttrString(rlcompleter, "Completer");
 
     if(Completer)
     {
@@ -612,7 +631,10 @@ QString PythonContext::LoadExtension(ICaptureContext &ctx, const rdcstr &extensi
 
   PyObject *sysobj = PyDict_GetItemString(main_dict, "sys");
 
-  PyObject *syspath = PyObject_GetAttrString(sysobj, "path");
+  PyObject *syspath = PyObject_SafeGetAttrString(sysobj, "path");
+
+  if(!syspath)
+    qCritical() << "couldn't get sys.path";
 
   // add extensions directories in
   for(QString p : PythonContext::GetApplicationExtensionsPaths())
@@ -631,7 +653,10 @@ QString PythonContext::LoadExtension(ICaptureContext &ctx, const rdcstr &extensi
 
   PyObject *ext = NULL;
 
-  current_global_handle = PyObject_GetAttrString(sysobj, "stdout");
+  current_global_handle = PyObject_SafeGetAttrString(sysobj, "stdout");
+
+  if(!syspath)
+    qCritical() << "couldn't get sys.stdout";
 
   if(extensions[extension] == NULL)
   {
@@ -643,7 +668,7 @@ QString PythonContext::LoadExtension(ICaptureContext &ctx, const rdcstr &extensi
     qInfo() << "Reloading " << QString(extension);
 
     // call unregister() if it exists
-    PyObject *unregister_func = PyObject_GetAttrString(extensions[extension], "unregister");
+    PyObject *unregister_func = PyObject_SafeGetAttrString(extensions[extension], "unregister");
 
     if(unregister_func)
     {
@@ -654,7 +679,10 @@ QString PythonContext::LoadExtension(ICaptureContext &ctx, const rdcstr &extensi
     }
 
     // if the extension is a package, we need to manually reload any loaded submodules
-    PyObject *sysmodules = PyObject_GetAttrString(sysobj, "modules");
+    PyObject *sysmodules = PyObject_SafeGetAttrString(sysobj, "modules");
+
+    if(!syspath)
+      qCritical() << "couldn't get sys.modules";
 
     PyObject *keys = PyDict_Keys(sysmodules);
 
@@ -721,7 +749,7 @@ QString PythonContext::LoadExtension(ICaptureContext &ctx, const rdcstr &extensi
   if(ext)
   {
     // if import succeeded, call register()
-    PyObject *register_func = PyObject_GetAttrString(ext, "register");
+    PyObject *register_func = PyObject_SafeGetAttrString(ext, "register");
 
     if(register_func)
     {
@@ -764,6 +792,8 @@ QString PythonContext::LoadExtension(ICaptureContext &ctx, const rdcstr &extensi
     }
     else
     {
+      qCritical() << "register() function not found in extension";
+      ret += tr("register() function not found in extension\n");
       ext = NULL;
     }
   }
@@ -776,25 +806,28 @@ QString PythonContext::LoadExtension(ICaptureContext &ctx, const rdcstr &extensi
   {
     FetchException(typeStr, valueStr, finalLine, frames);
 
-    ret += lit("\n");
-
     ret = ret.trimmed();
 
-    qCritical("Error importing extension module. %s: %s", typeStr.toUtf8().data(),
-              valueStr.toUtf8().data());
-    ret += tr("Error importing extension module. %1: %2\n\n").arg(typeStr).arg(valueStr);
+    ret += lit("\n");
 
-    if(!frames.isEmpty())
+    if(!valueStr.isEmpty())
     {
-      qCritical() << "Traceback (most recent call last):";
-      ret += tr("Traceback (most recent call last):\n");
-      for(const QString &f : frames)
+      qCritical("Error importing extension module. %s: %s", typeStr.toUtf8().data(),
+                valueStr.toUtf8().data());
+      ret += tr("Error importing extension module. %1: %2\n\n").arg(typeStr).arg(valueStr);
+
+      if(!frames.isEmpty())
       {
-        QStringList lines = f.split(QLatin1Char('\n'));
-        for(const QString &line : lines)
+        qCritical() << "Traceback (most recent call last):";
+        ret += tr("Traceback (most recent call last):\n");
+        for(const QString &f : frames)
         {
-          qCritical("  %s", line.toUtf8().data());
-          ret += line + lit("\n");
+          QStringList lines = f.split(QLatin1Char('\n'));
+          for(const QString &line : lines)
+          {
+            qCritical("  %s", line.toUtf8().data());
+            ret += line + lit("\n");
+          }
         }
       }
     }
@@ -1060,7 +1093,10 @@ QStringList PythonContext::completionOptions(QString base)
 
   PyGILState_STATE gil = PyGILState_Ensure();
 
-  PyObject *completeFunction = PyObject_GetAttrString(m_Completer, "complete");
+  PyObject *completeFunction = PyObject_SafeGetAttrString(m_Completer, "complete");
+
+  if(!completeFunction)
+    return ret;
 
   int idx = 0;
   PyObject *opt = NULL;
@@ -1397,7 +1433,7 @@ extern "C" PyObject *GetCurrentGlobalHandle()
   PyObject *sys = PyImport_ImportModule("sys");
   if(sys)
   {
-    PyObject *ret = PyObject_GetAttrString(sys, "stdout");
+    PyObject *ret = PyObject_SafeGetAttrString(sys, "stdout");
     Py_XDECREF(sys);
     return ret;
   }