From a2dd176bc22a2e1388f591fe9bda16afcf283eab Mon Sep 17 00:00:00 2001
From: wmayer <wmayer@users.sourceforge.net>
Date: Thu, 27 Apr 2017 15:09:28 +0200
Subject: [PATCH] issue #0002996: PyObjectBase notification chain can lead to
 unexpected changes to document

---
 src/Base/PyObjectBase.cpp | 67 ++++++++++++++++++++-------------------
 src/Mod/Test/Document.py  | 36 +++++++++++++++++++++
 2 files changed, 71 insertions(+), 32 deletions(-)

diff --git a/src/Base/PyObjectBase.cpp b/src/Base/PyObjectBase.cpp
index ef4b855c70..81741839b9 100644
--- a/src/Base/PyObjectBase.cpp
+++ b/src/Base/PyObjectBase.cpp
@@ -292,16 +292,18 @@ void PyObjectBase::resetAttribute()
     if (attrDict) {
         // This is the attribute name to the parent structure
         // which we search for in the dict
-        PyObject* key = PyString_FromString("__attribute_of_parent__");
-        PyObject* attr = PyDict_GetItem(attrDict, key);
+        PyObject* key1 = PyString_FromString("__attribute_of_parent__");
+        PyObject* key2 = PyString_FromString("__instance_of_parent__");
+        PyObject* attr = PyDict_GetItem(attrDict, key1);
+        PyObject* inst = PyDict_GetItem(attrDict, key2);
         if (attr) {
-            PyObject* parent = PyDict_GetItem(attrDict, attr);
-            if (parent) {
-                PyDict_DelItem(attrDict, attr);
-            }
-            PyDict_DelItem(attrDict, key);
+            PyDict_DelItem(attrDict, key1);
         }
-        Py_DECREF(key);
+        if (inst) {
+            PyDict_DelItem(attrDict, key2);
+        }
+        Py_DECREF(key1);
+        Py_DECREF(key2);
     }
 }
 
@@ -311,12 +313,14 @@ void PyObjectBase::setAttributeOf(const char* attr, PyObject* par)
         attrDict = PyDict_New();
     }
 
-    PyObject* key = PyString_FromString("__attribute_of_parent__");
+    PyObject* key1 = PyString_FromString("__attribute_of_parent__");
+    PyObject* key2 = PyString_FromString("__instance_of_parent__");
     PyObject* attro = PyString_FromString(attr);
-    PyDict_SetItem(attrDict, key, attro);
-    PyDict_SetItem(attrDict, attro, par);
+    PyDict_SetItem(attrDict, key1, attro);
+    PyDict_SetItem(attrDict, key2, par);
     Py_DECREF(attro);
-    Py_DECREF(key);
+    Py_DECREF(key1);
+    Py_DECREF(key2);
 }
 
 void PyObjectBase::startNotify()
@@ -324,30 +328,29 @@ void PyObjectBase::startNotify()
     if (attrDict) {
         // This is the attribute name to the parent structure
         // which we search for in the dict
-        PyObject* key = PyString_FromString("__attribute_of_parent__");
-        PyObject* attr = PyDict_GetItem(attrDict, key);
-        if (attr) {
-            PyObject* parent = PyDict_GetItem(attrDict, attr);
-            if (parent) {
-                // Inside __setattr of the parent structure the 'attr'
-                // is being removed from the dict and thus its reference
-                // counter will be decremented. To avoid to be deleted we
-                // must tmp. increment it and afterwards decrement it again.
-                Py_INCREF(parent);
-                Py_INCREF(attr);
-                Py_INCREF(this);
+        PyObject* key1 = PyString_FromString("__attribute_of_parent__");
+        PyObject* key2 = PyString_FromString("__instance_of_parent__");
+        PyObject* attr = PyDict_GetItem(attrDict, key1);
+        PyObject* parent = PyDict_GetItem(attrDict, key2);
+        if (attr && parent) {
+            // Inside __setattr of the parent structure the 'attr'
+            // is being removed from the dict and thus its reference
+            // counter will be decremented. To avoid to be deleted we
+            // must tmp. increment it and afterwards decrement it again.
+            Py_INCREF(parent);
+            Py_INCREF(attr);
+            Py_INCREF(this);
 
-                __setattr(parent, PyString_AsString(attr), this);
+            __setattr(parent, PyString_AsString(attr), this);
 
-                Py_DECREF(parent); // might be destroyed now
-                Py_DECREF(attr); // might be destroyed now
-                Py_DECREF(this); // might be destroyed now
+            Py_DECREF(parent); // might be destroyed now
+            Py_DECREF(attr); // might be destroyed now
+            Py_DECREF(this); // might be destroyed now
 
-                if (PyErr_Occurred())
-                    PyErr_Clear();
-            }
+            if (PyErr_Occurred())
+                PyErr_Clear();
         }
-        Py_DECREF(key);
+        Py_DECREF(key1);
     }
 }
 
diff --git a/src/Mod/Test/Document.py b/src/Mod/Test/Document.py
index 3e4193dd0e..3ae2c8def6 100644
--- a/src/Mod/Test/Document.py
+++ b/src/Mod/Test/Document.py
@@ -364,6 +364,42 @@ class DocumentBasicCases(unittest.TestCase):
     o.Placement.Base.x=5
     self.assertEqual(o.Placement.Base.x, 5)
 
+  def testNotification_Issue2996(self):
+    if not FreeCAD.GuiUp:
+      return
+    # works only if Gui is shown
+    class ViewProvider:
+      def __init__(self, vobj):
+        vobj.Proxy=self
+
+      def attach(self, vobj):
+        self.ViewObject = vobj
+        self.Object = vobj.Object
+
+      def claimChildren(self):
+        children = [self.Object.Link]
+        return children
+
+    obj=self.Doc.addObject("App::FeaturePython", "Sketch")
+    obj.addProperty("App::PropertyLink","Link")
+    ViewProvider(obj.ViewObject)
+
+    ext=self.Doc.addObject("App::FeatureTest", "Extrude")
+    ext.Link=obj
+
+    sli=self.Doc.addObject("App::FeaturePython", "Slice")
+    sli.addProperty("App::PropertyLink","Link").Link=ext
+    ViewProvider(sli.ViewObject)
+
+    com=self.Doc.addObject("App::FeaturePython", "CompoundFilter")
+    com.addProperty("App::PropertyLink", "Link").Link=sli
+    ViewProvider(com.ViewObject)
+
+    ext.Label="test"
+
+    self.assertEqual(ext.Link, obj)
+    self.assertNotEqual(ext.Link, sli)
+
   def tearDown(self):
     #closing doc
     FreeCAD.closeDocument("CreateTest")