From 1f83094b4430dfaa6fbd2e2b2381bf5332bba109 Mon Sep 17 00:00:00 2001
From: wmayer <wmayer@users.sourceforge.net>
Date: Thu, 17 Nov 2022 19:05:32 +0100
Subject: [PATCH] App: fix endless-loop in DocumentObject::getParents

---
 src/App/DocumentObject.cpp | 76 ++++++++++++++++++++++++++------------
 src/App/DocumentObject.h   |  3 ++
 2 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/src/App/DocumentObject.cpp b/src/App/DocumentObject.cpp
index a9af86bb32..10b6fccf5e 100644
--- a/src/App/DocumentObject.cpp
+++ b/src/App/DocumentObject.cpp
@@ -96,10 +96,9 @@ DocumentObject::~DocumentObject()
     }
 }
 
-App::DocumentObjectExecReturn *DocumentObject::recompute()
+void DocumentObject::printInvalidLinks() const
 {
-    //check if the links are valid before making the recompute
-    if(!GeoFeatureGroupExtension::areLinksValid(this)) {
+    try {
         // Get objects that have invalid link scope, and print their names.
         // Truncate the invalid object list name strings for readability, if they happen to be very long.
         std::vector<App::DocumentObject*> invalid_linkobjs;
@@ -113,25 +112,44 @@ App::DocumentObjectExecReturn *DocumentObject::recompute()
                     scopenames += "... ";
                     break;
                 }
+
                 scopenames += scope.first->getNameInDocument();
                 scopenames += " ";
             }
+
             if (objnames.length() > 80) {
                 objnames += "... ";
                 break;
             }
         }
+
         if (objnames.empty()) {
             objnames = "N/A";
-        } else {
+        }
+        else {
             objnames.pop_back();
         }
+
         if (scopenames.empty()) {
             scopenames = "N/A";
-        } else {
+        }
+        else {
             scopenames.pop_back();
         }
-        Base::Console().Warning("%s: Link(s) to object(s) '%s' go out of the allowed scope '%s'. Instead, the linked object(s) reside within '%s'.\n", getTypeId().getName(), objnames.c_str(), getNameInDocument(), scopenames.c_str());
+
+        Base::Console().Warning("%s: Link(s) to object(s) '%s' go out of the allowed scope '%s'. Instead, the linked object(s) reside within '%s'.\n",
+                                getTypeId().getName(), objnames.c_str(), getNameInDocument(), scopenames.c_str());
+    }
+    catch (const Base::Exception& e) {
+        e.ReportException();
+    }
+}
+
+App::DocumentObjectExecReturn *DocumentObject::recompute()
+{
+    //check if the links are valid before making the recompute
+    if (!GeoFeatureGroupExtension::areLinksValid(this)) {
+        printInvalidLinks();
     }
 
     // set/unset the execution bit
@@ -829,31 +847,43 @@ std::vector<std::string> DocumentObject::getSubObjects(int reason) const {
     return ret;
 }
 
-std::vector<std::pair<App::DocumentObject *,std::string> > DocumentObject::getParents(int depth) const {
-    std::vector<std::pair<App::DocumentObject *,std::string> > ret;
-    if(!getNameInDocument() || !GetApplication().checkLinkDepth(depth))
+std::vector<std::pair<App::DocumentObject *,std::string>> DocumentObject::getParents(int depth) const {
+    std::vector<std::pair<App::DocumentObject *, std::string>> ret;
+    if (!getNameInDocument() || !GetApplication().checkLinkDepth(depth, MessageOption::Throw)) {
         return ret;
+    }
+
     std::string name(getNameInDocument());
     name += ".";
-    for(auto parent : getInList()) {
-        if(!parent || !parent->getNameInDocument())
-            continue;
-        if(!parent->hasChildElement() && 
-           !parent->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId()))
-            continue;
-        if(!parent->getSubObject(name.c_str()))
+    for (auto parent : getInList()) {
+        if (!parent || !parent->getNameInDocument()) {
             continue;
+        }
 
-        auto links = GetApplication().getLinksTo(parent,App::GetLinkRecursive);
+        if (!parent->hasChildElement() &&
+            !parent->hasExtension(GeoFeatureGroupExtension::getExtensionClassTypeId())) {
+            continue;
+        }
+
+        if (!parent->getSubObject(name.c_str())) {
+            continue;
+        }
+
+        auto links = GetApplication().getLinksTo(parent, App::GetLinkRecursive);
         links.insert(parent);
-        for(auto parent : links) {
-            auto parents = parent->getParents(depth+1);
-            if(parents.empty()) 
-                parents.emplace_back(parent,std::string());
-            for(auto &v : parents) 
-                ret.emplace_back(v.first,v.second+name);
+
+        for (auto parent : links) {
+            auto parents = parent->getParents(depth + 1);
+            if (parents.empty()) {
+                parents.emplace_back(parent, std::string());
+            }
+
+            for (auto &v : parents) {
+                ret.emplace_back(v.first, v.second + name);
+            }
         }
     }
+
     return ret;
 }
 
diff --git a/src/App/DocumentObject.h b/src/App/DocumentObject.h
index e863958edc..dee7fff9ad 100644
--- a/src/App/DocumentObject.h
+++ b/src/App/DocumentObject.h
@@ -614,6 +614,9 @@ protected:
     /// get called when a property status has changed
     void onPropertyStatusChanged(const Property &prop, unsigned long oldStatus) override;
 
+private:
+    void printInvalidLinks() const;
+
      /// python object of this class and all descendent
 protected: // attributes
     Py::SmartPtr PythonObject;