From 15b61112b1c059ddcbc8ddffb0a9906a5eeb6786 Mon Sep 17 00:00:00 2001
From: "Zheng, Lei" <realthunder.dev@gmail.com>
Date: Wed, 21 Aug 2019 10:01:21 +0800
Subject: [PATCH] App: prevent recursive abort/commitTransaction

---
 src/App/Document.cpp | 55 +++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/src/App/Document.cpp b/src/App/Document.cpp
index 748b7e8c5d..1d96a603cf 100644
--- a/src/App/Document.cpp
+++ b/src/App/Document.cpp
@@ -175,6 +175,7 @@ struct DocumentP
     int iTransactionMode;
     bool rollback;
     bool undoing; ///< document in the middle of undo or redo
+    bool committing;
     std::bitset<32> StatusBits;
     int iUndoMode;
     unsigned int UndoMemSize;
@@ -200,6 +201,7 @@ struct DocumentP
         iTransactionMode = 0;
         rollback = false;
         undoing = false;
+        committing = false;
         StatusBits.set((size_t)Document::Closable, true);
         StatusBits.set((size_t)Document::KeepTrailingDigits, true);
         StatusBits.set((size_t)Document::Restoring, false);
@@ -1044,8 +1046,9 @@ std::vector<std::string> Document::getAvailableRedoNames() const
 }
 
 void Document::openTransaction(const char* name) {
-    if(isPerformingTransaction()) {
-        FC_WARN("Cannot open transaction while transacting");
+    if(isPerformingTransaction() || d->committing) {
+        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG))
+            FC_WARN("Cannot open transaction while transacting");
         return;
     }
 
@@ -1054,8 +1057,9 @@ void Document::openTransaction(const char* name) {
 
 int Document::_openTransaction(const char* name, int id)
 {
-    if(isPerformingTransaction()) {
-        FC_WARN("Cannot open transaction while transacting");
+    if(isPerformingTransaction() || d->committing) {
+        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG))
+            FC_WARN("Cannot open transaction while transacting");
         return 0;
     }
 
@@ -1146,7 +1150,7 @@ void Document::_checkTransaction(DocumentObject* pcDelObj, const Property *What,
 
 void Document::_clearRedos()
 {
-    if(isPerformingTransaction()) {
+    if(isPerformingTransaction() || d->committing) {
         FC_ERR("Cannot clear redo while transacting");
         return;
     }
@@ -1159,8 +1163,9 @@ void Document::_clearRedos()
 }
 
 void Document::commitTransaction() {
-    if(isPerformingTransaction()) {
-        FC_WARN("Cannot commit transaction while transacting");
+    if(isPerformingTransaction() || d->committing) {
+        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG))
+            FC_WARN("Cannot commit transaction while transacting");
         return;
     }
 
@@ -1170,11 +1175,13 @@ void Document::commitTransaction() {
 
 void Document::_commitTransaction(bool notify)
 {
-    if(isPerformingTransaction()) {
-        FC_WARN("Cannot commit transaction while transacting");
+    if(isPerformingTransaction() || d->committing) {
+        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG))
+            FC_WARN("Cannot commit transaction while transacting");
         return;
     }
     if (d->activeUndoTransaction) {
+        Base::FlagToggler<> flag(d->committing);
         Application::TransactionSignaller signaller(false,true);
         int id = d->activeUndoTransaction->getID();
         mUndoTransactions.push_back(d->activeUndoTransaction);
@@ -1193,8 +1200,9 @@ void Document::_commitTransaction(bool notify)
 }
 
 void Document::abortTransaction() {
-    if(isPerformingTransaction()) {
-        FC_WARN("Cannot abort transaction while transacting");
+    if(isPerformingTransaction() || d->committing) {
+        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG))
+            FC_WARN("Cannot abort transaction while transacting");
         return;
     }
     if (d->activeUndoTransaction) 
@@ -1203,18 +1211,17 @@ void Document::abortTransaction() {
 
 void Document::_abortTransaction()
 {
-    if(isPerformingTransaction()) {
-        FC_WARN("Cannot abort transaction while transacting");
-        return;
+    if(isPerformingTransaction() || d->committing) {
+        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG))
+            FC_WARN("Cannot abort transaction while transacting");
     }
 
-    if (d->activeUndoTransaction) {
+    if (!d->activeUndoTransaction) {
+        Base::FlagToggler<bool> flag(d->rollback);
         Application::TransactionSignaller signaller(true,true);
-        {
-            Base::FlagToggler<bool> flag(d->rollback);
-            // applying the so far made changes
-            d->activeUndoTransaction->apply(*this,false);
-        }
+
+        // applying the so far made changes
+        d->activeUndoTransaction->apply(*this,false);
 
         // destroy the undo
         mUndoMap.erase(d->activeUndoTransaction->getID());
@@ -1263,7 +1270,7 @@ bool Document::isTransactionEmpty() const
 
 void Document::clearUndos()
 {
-    if(isPerformingTransaction()) {
+    if(isPerformingTransaction() || d->committing) {
         FC_ERR("Cannot clear undos while transacting");
         return;
     }
@@ -3034,6 +3041,12 @@ int Document::recompute(const std::vector<App::DocumentObject*> &objs, bool forc
 
 int Document::recompute(const std::vector<App::DocumentObject*> &objs, bool force, bool *hasError, int options) 
 {
+    if (d->undoing || d->rollback) {
+        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG))
+            FC_WARN("Ignore document recompute on undo/redo");
+        return 0;
+    }
+
     int objectCount = 0;
     if (testStatus(Document::PartialDoc)) {
         if(mustExecute())