From 3b87c991cb72882a2b9c96dc925a654826c105a2 Mon Sep 17 00:00:00 2001
From: Kevin Martin <kpmartin@papertrail.ca>
Date: Wed, 5 Mar 2025 13:33:49 -0500
Subject: [PATCH 1/2] Deregister DocumentObject's Label before deleting the
 object

Under certain circumstances the existing code would try to obtain the Label of a DocumentObject after that object had been deleted, causing an access to freed memory and failing to deregister the Label. The latter would lead to phanton Label collisions and unexpected generation of unique labels.
---
 src/App/Document.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/App/Document.cpp b/src/App/Document.cpp
index ecf12cc3a7..d843343c9b 100644
--- a/src/App/Document.cpp
+++ b/src/App/Document.cpp
@@ -3758,6 +3758,7 @@ void Document::removeObject(const char* sName)
     d->objectIdMap.erase(pos->second->_Id);
     // Unset the bit to be on the safe side
     pos->second->setStatus(ObjectStatus::Remove, false);
+    unregisterLabel(pos->second->Label.getStrValue());
 
     // do no transactions if we do a rollback!
     std::unique_ptr<DocumentObject> tobedestroyed;
@@ -3775,7 +3776,6 @@ void Document::removeObject(const char* sName)
         }
     }
 
-    unregisterLabel(pos->second->Label.getStrValue());
     for (std::vector<DocumentObject*>::iterator obj = d->objectArray.begin();
          obj != d->objectArray.end();
          ++obj) {
@@ -3855,6 +3855,11 @@ void Document::_removeObject(DocumentObject* pcObject)
         signalTransactionRemove(*pcObject, 0);
         breakDependency(pcObject, true);
     }
+    // TODO: Transaction::addObjectName could potentially have freed (deleted) pcObject so some of the following
+    // code may be dereferencing a pointer to a deleted object which is not legal. if (d->rollback) this does not occur
+    // and instead pcObject is deleted at the end of this function.
+    // This either should be fixed, perhaps by moving the following lines up in the code,
+    // or there should be a comment explaining why the object will never be deleted because of the logic that got us here.
 
     // remove from map
     pcObject->setStatus(ObjectStatus::Remove, false);  // Unset the bit to be on the safe side

From e864b17a5a04bf38853cdfe95cb356e5ce019678 Mon Sep 17 00:00:00 2001
From: Kevin Martin <kpmartin@papertrail.ca>
Date: Wed, 5 Mar 2025 13:36:35 -0500
Subject: [PATCH 2/2] Only do Label Change notification if the label truly
 changes

The code for setting the value of a Label property would do a quick return if the new label was equal to the current one, but otherwise the proposed new label might be modified to make it unique. If the modified new value were equal to the current value, a Change notification would still be generated. This no longer occurs.
---
 src/App/PropertyStandard.cpp | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/App/PropertyStandard.cpp b/src/App/PropertyStandard.cpp
index ac6fe61e51..c1c8e2c95c 100644
--- a/src/App/PropertyStandard.cpp
+++ b/src/App/PropertyStandard.cpp
@@ -1425,23 +1425,27 @@ PropertyString::PropertyString() = default;
 
 PropertyString::~PropertyString() = default;
 
-void PropertyString::setValue(const char* newLabel)
+void PropertyString::setValue(const char* newValue)
 {
-    if (!newLabel) {
+    if (!newValue) {
         return;
     }
 
-    if (_cValue == newLabel) {
+    if (_cValue == newValue) {
         return;
     }
 
     std::vector<std::pair<Property*, std::unique_ptr<Property>>> propChanges;
-    std::string label = newLabel;
+    std::string newValueStr = newValue;
     auto obj = dynamic_cast<DocumentObject*>(getContainer());
     bool commit = false;
 
     if (obj && this == &obj->Label) {
-        propChanges = obj->onProposedLabelChange(label);
+        propChanges = obj->onProposedLabelChange(newValueStr);
+        if (_cValue == newValueStr) {
+            // OnProposedLabelChange has changed the new value to what the current value is
+            return;
+        }
         if (!propChanges.empty() && !GetApplication().getActiveTransaction()) {
             commit = true;
             std::ostringstream str;
@@ -1451,7 +1455,7 @@ void PropertyString::setValue(const char* newLabel)
     }
 
     aboutToSetValue();
-    _cValue = label;
+    _cValue = newValueStr;
     hasSetValue();
 
     for (auto& change : propChanges) {