Improve three::_findSubclassOf resiliency based on two's code. (#3990)

rtloader/three: mimic the two's implemention of findSubclassof.

rtloader/three/three.cpp

@@ -504,70 +504,86 @@ PyObject *Three::_importFrom(const char *module, const char *name)
 
 PyObject *Three::_findSubclassOf(PyObject *base, PyObject *module)
 {
+    // baseClass is not a Class type
     if (base == NULL || !PyType_Check(base)) {
         setError("base class is not of type 'Class'");
         return NULL;
     }
 
+    // module is not a Module object
     if (module == NULL || !PyModule_Check(module)) {
         setError("module is not of type 'Module'");
         return NULL;
     }
 
     PyObject *dir = PyObject_Dir(module);
     if (dir == NULL) {
+        PyErr_Clear();
         setError("there was an error calling dir() on module object");
         return NULL;
     }
 
     PyObject *klass = NULL;
     for (int i = 0; i < PyList_GET_SIZE(dir); i++) {
-        // get symbol name
-        char *symbol_name;
-        PyObject *symbol = PyList_GetItem(dir, i);
-        if (symbol != NULL) {
-            symbol_name = as_string(symbol);
-            if (symbol_name == NULL)
-                continue;
-        } else {
-            // Gets exception reason
-            PyObject *reason = PyUnicodeDecodeError_GetReason(PyExc_IndexError);
+        // Reset `klass` at every iteration so its state is always clean when we
+        // continue the loop or return early. Reset at first iteration is useless
+        // but it keeps the code readable.
+        Py_XDECREF(klass);
+        klass = NULL;
 
-            // Clears exception and sets error
-            PyException_SetTraceback(PyExc_IndexError, Py_None);
-            setError((const char *)PyBytes_AsString(reason));
+        // get the symbol in current list item
+        PyObject *symbol = PyList_GetItem(dir, i);
+        if (symbol == NULL) {
+            // This should never happen as it means we're out of bounds
+            PyErr_Clear();
+            setError("there was an error browsing dir() output");
             goto done;
         }
 
+        // get symbol name, as_string returns a new object that we have to free
+        char *symbol_name = as_string(symbol);
+        if (symbol_name == NULL) {
+            // as_string returns NULL if `symbol` is not a string object
+            // and raises TypeError. Let's clear the error and keep going.
+            PyErr_Clear();
+            continue;
+        }
+
+        // get symbol instance. It's a new ref but in case of success we don't
+        // DecRef since we return it and the caller will be owner
         klass = PyObject_GetAttrString(module, symbol_name);
         ::_free(symbol_name);
         if (klass == NULL) {
+            PyErr_Clear();
             continue;
         }
 
         // Not a class, ignore
         if (!PyType_Check(klass)) {
-            Py_XDECREF(klass);
             continue;
         }
 
         // Unrelated class, ignore
         if (!PyType_IsSubtype((PyTypeObject *)klass, (PyTypeObject *)base)) {
-            Py_XDECREF(klass);
             continue;
         }
 
-        // `klass` is actually `base` itself, ignore
-        if (PyObject_RichCompareBool(klass, base, Py_EQ) == 1) {
-            Py_XDECREF(klass);
+        // check whether `klass` is actually `base` itself
+        int retval = PyObject_RichCompareBool(klass, base, Py_EQ);
+        if (retval == 1) {
+            // `klass` is indeed `base`, continue
+            continue;
+        } else if (retval == -1) {
+            // an error occurred calling __eq__, clear and continue
+            PyErr_Clear();
             continue;
         }
 
         // does `klass` have subclasses?
         char func_name[] = "__subclasses__";
         PyObject *children = PyObject_CallMethod(klass, func_name, NULL);
         if (children == NULL) {
-            Py_XDECREF(klass);
+            PyErr_Clear();
             continue;
         }
 
@@ -577,19 +593,21 @@ PyObject *Three::_findSubclassOf(PyObject *base, PyObject *module)
 
         // Agent integrations are supposed to have no subclasses
         if (children_count > 0) {
-            Py_XDECREF(klass);
             continue;
         }
 
         // got it, return the check class
         goto done;
     }
 
+    // we couldn't find any good subclass, set an error and reset
+    // `klass` state for one last time before moving to `done`.
     setError("cannot find a subclass");
+    Py_XDECREF(klass);
     klass = NULL;
 
 done:
-    Py_DECREF(dir);
+    Py_XDECREF(dir);
     return klass;
 }