From e4770ffa9e2101a1beea4cfbeffb6927945073d0 Mon Sep 17 00:00:00 2001
From: wmayer <wmayer@users.sourceforge.net>
Date: Mon, 17 Jul 2017 18:24:10 +0200
Subject: [PATCH] protect document against nested recomputes + convert
 ObjectStatusLocker into a template class to make its usage more flexible +
 add status flag 'Recomputing' and set in Document::recompute to detect and
 avoid nested calls of recompute

---
 src/App/Document.cpp       |  7 +++++++
 src/App/Document.h         |  3 ++-
 src/App/DocumentObject.cpp |  2 +-
 src/App/DocumentObject.h   |  9 +++++----
 src/App/DocumentPyImp.cpp  | 12 +++++++++---
 src/App/FeaturePython.cpp  |  4 ++--
 6 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/App/Document.cpp b/src/App/Document.cpp
index eff7d899a7..29653c8b68 100644
--- a/src/App/Document.cpp
+++ b/src/App/Document.cpp
@@ -2007,6 +2007,11 @@ void Document::renameObjectIdentifiers(const std::map<App::ObjectIdentifier, App
 #ifdef USE_OLD_DAG
 int Document::recompute()
 {
+    if (testStatus(Document::Recomputing)) {
+        // this is clearly a bug in the calling instance
+        throw Base::RuntimeError("Nested recomputes of a document are not allowed");
+    }
+
     int objectCount = 0;
     
     // The 'SkipRecompute' flag can be (tmp.) set to avoid to many
@@ -2015,6 +2020,8 @@ int Document::recompute()
     if (skip)
         return 0;
 
+    ObjectStatusLocker<Document::Status, Document> exe(Document::Recomputing, this);
+
     // delete recompute log
     for (std::vector<App::DocumentObjectExecReturn*>::iterator it=_RecomputeLog.begin();it!=_RecomputeLog.end();++it)
         delete *it;
diff --git a/src/App/Document.h b/src/App/Document.h
index 351d6388da..01f140ddf1 100644
--- a/src/App/Document.h
+++ b/src/App/Document.h
@@ -66,7 +66,8 @@ public:
         SkipRecompute = 0,
         KeepTrailingDigits = 1,
         Closable = 2,
-        Restoring = 3
+        Restoring = 3,
+        Recomputing = 4
     };
 
     /** @name Properties */
diff --git a/src/App/DocumentObject.cpp b/src/App/DocumentObject.cpp
index 0099787415..fafc5a83e9 100644
--- a/src/App/DocumentObject.cpp
+++ b/src/App/DocumentObject.cpp
@@ -74,7 +74,7 @@ DocumentObject::~DocumentObject(void)
 App::DocumentObjectExecReturn *DocumentObject::recompute(void)
 {
     // set/unset the execution bit
-    ObjectStatusLocker exe(App::Recompute, this);
+    ObjectStatusLocker<ObjectStatus, DocumentObject> exe(App::Recompute, this);
     return this->execute();
 }
 
diff --git a/src/App/DocumentObject.h b/src/App/DocumentObject.h
index b0829edc89..8d543fd5b5 100644
--- a/src/App/DocumentObject.h
+++ b/src/App/DocumentObject.h
@@ -295,16 +295,17 @@ private:
     bool _isInOutListRecursive(const DocumentObject *act, const DocumentObject* test, const DocumentObject* checkObj, int depth) const;
 };
 
-class AppExport ObjectStatusLocker
+template<typename Status, class Object>
+class ObjectStatusLocker
 {
 public:
-    ObjectStatusLocker(ObjectStatus s, DocumentObject* o) : status(s), obj(o)
+    ObjectStatusLocker(Status s, Object* o) : status(s), obj(o)
     { obj->setStatus(status, true); }
     ~ObjectStatusLocker()
     { obj->setStatus(status, false); }
 private:
-    ObjectStatus status;
-    DocumentObject* obj;
+    Status status;
+    Object* obj;
 };
 
 } //namespace App
diff --git a/src/App/DocumentPyImp.cpp b/src/App/DocumentPyImp.cpp
index 249fe4e0a9..e3d2303df4 100644
--- a/src/App/DocumentPyImp.cpp
+++ b/src/App/DocumentPyImp.cpp
@@ -403,9 +403,15 @@ PyObject*  DocumentPy::clearUndos(PyObject * args)
 PyObject*  DocumentPy::recompute(PyObject * args)
 {
     if (!PyArg_ParseTuple(args, ""))     // convert args: Python->C 
-        return NULL;                    // NULL triggers exception 
-    int objectCount = getDocumentPtr()->recompute();
-    return Py::new_reference_to(Py::Int(objectCount));
+        return NULL;                    // NULL triggers exception
+    try {
+        int objectCount = getDocumentPtr()->recompute();
+        return Py::new_reference_to(Py::Int(objectCount));
+    }
+    catch (const Base::RuntimeError& e) {
+        PyErr_SetString(PyExc_RuntimeError, e.what());
+        return 0;
+    }
 }
 
 PyObject*  DocumentPy::getObject(PyObject *args)
diff --git a/src/App/FeaturePython.cpp b/src/App/FeaturePython.cpp
index eff825605b..bafd32767d 100644
--- a/src/App/FeaturePython.cpp
+++ b/src/App/FeaturePython.cpp
@@ -63,7 +63,7 @@ bool FeaturePythonImp::execute()
             Py::Object feature = static_cast<PropertyPythonObject*>(proxy)->getValue();
             if (feature.hasAttr(std::string("execute"))) {
                 if (feature.hasAttr("__object__")) {
-                    ObjectStatusLocker exe(App::PythonCall, object);
+                    ObjectStatusLocker<ObjectStatus, DocumentObject> exe(App::PythonCall, object);
                     Py::Callable method(feature.getAttr(std::string("execute")));
                     Py::Tuple args;
                     Py::Object res = method.apply(args);
@@ -72,7 +72,7 @@ bool FeaturePythonImp::execute()
                     return true;
                 }
                 else {
-                    ObjectStatusLocker exe(App::PythonCall, object);
+                    ObjectStatusLocker<ObjectStatus, DocumentObject> exe(App::PythonCall, object);
                     Py::Callable method(feature.getAttr(std::string("execute")));
                     Py::Tuple args(1);
                     args.setItem(0, Py::Object(object->getPyObject(), true));