Sketcher: Fix crash on constraint rename

========================================

Report:
https://github.com/FreeCAD/FreeCAD/pull/4183
https://github.com/realthunder/FreeCAD_assembly3/issues/387

Problem:
renameConstraint() previously implemented exclusively in SketchObjectPyImp.cpp,
will change the Constraints property without updating the solver. A prospective
drag operation would rely on a deleted pointer constraint which leads to the
crash.

Solution:
- mark the solver status as needing an update
- leverage new through sketchobject r/w interface to ensure solver is synchronised
before the temporary move operation starts

Bonus:
move the core of the function to SketchObject.cpp so that input data validity
check on constraint change is inhibited.

src/Mod/Sketcher/App/SketchObject.cpp

@@ -8002,6 +8002,27 @@ int SketchObject::autoRemoveRedundants(bool updategeo)
     return redundants.size();
 }
 
+int SketchObject::renameConstraint(int GeoId, std::string name)
+{
+    // only change the constraint item if the names are different
+    const Constraint* item = Constraints[GeoId];
+
+    if (item->Name != name) {
+        Base::StateLocker lock(managedoperation, true); // no need to check input data validity as this is an sketchobject managed operation.
+
+        Constraint* copy = item->clone();
+        copy->Name = name;
+
+        Constraints.set1Value(GeoId, copy);
+        delete copy;
+
+        solverNeedsUpdate = true; // make sure any prospective solver access updates the constraint pointer that just got invalidated
+
+        return 0;
+    }
+    return -1;
+}
+
 std::vector<Base::Vector3d> SketchObject::getOpenVertices(void) const
 {
     std::vector<Base::Vector3d> points;

src/Mod/Sketcher/App/SketchObject.h

@@ -361,11 +361,12 @@ public: /* Solver exposed interface */
     inline void setRecalculateInitialSolutionWhileMovingPoint(bool recalculateInitialSolutionWhileMovingPoint)
         {solvedSketch.setRecalculateInitialSolutionWhileMovingPoint(recalculateInitialSolutionWhileMovingPoint);}
     /// Forwards a request for a temporary initMove to the solver using the current sketch state as a reference (enables dragging)
-    inline int initTemporaryMove(int geoId, PointPos pos, bool fine=true)
-        { return solvedSketch.initMove(geoId,pos,fine);}
-    /// Forwards a request for point or curve temporary movement to the solver using the current state as a reference (enables dragging)
-    inline int moveTemporaryPoint(int geoId, PointPos pos, Base::Vector3d toPoint, bool relative=false)
-        { return solvedSketch.movePoint(geoId, pos, toPoint, relative);}
+    inline int initTemporaryMove(int geoId, PointPos pos, bool fine=true);
+    /** Forwards a request for point or curve temporary movement to the solver using the current state as a reference (enables dragging).
+     *  NOTE: A temporary move operation must always be preceded by a initTemporaryMove() operation.
+     */
+    inline int moveTemporaryPoint(int geoId, PointPos pos, Base::Vector3d toPoint, bool relative=false);
+    /// forwards a request to update an extension of a geometry of the solver to the solver.
     inline void updateSolverExtension(int geoId, std::unique_ptr<Part::GeometryExtension> && ext)
         { return solvedSketch.updateExtension(geoId, std::move(ext));}
 
@@ -436,6 +437,8 @@ public:
     /// returns the number of redundant constraints detected
     int autoRemoveRedundants(bool updategeo = true);
 
+    int renameConstraint(int GeoId, std::string name);
+
     // Validation routines
     std::vector<Base::Vector3d> getOpenVertices(void) const;
 
@@ -527,6 +530,22 @@ private:
     bool managedoperation; // indicates whether changes to properties are the deed of SketchObject or not (for input validation)
 };
 
+inline int SketchObject::initTemporaryMove(int geoId, PointPos pos, bool fine/*=true*/)
+{
+    // if a previous operation did not update the geometry (including geometry extensions)
+    // or constraints (including any deleted pointer, as in renameConstraint) of the solver,
+    // here we update them before starting a temporary operation.
+    if(solverNeedsUpdate)
+        solve();
+
+    return solvedSketch.initMove(geoId,pos,fine);
+}
+
+inline int SketchObject::moveTemporaryPoint(int geoId, PointPos pos, Base::Vector3d toPoint, bool relative/*=false*/)
+{
+    return solvedSketch.movePoint(geoId, pos, toPoint, relative);
+}
+
 typedef App::FeaturePythonT<SketchObject> SketchObjectPython;
 
 } //namespace Sketcher

src/Mod/Sketcher/App/SketchObjectPyImp.cpp

@@ -417,14 +417,8 @@ PyObject* SketchObjectPy::renameConstraint(PyObject *args)
         }
     }
 
-    // only change the constraint item if the names are different
-    const Constraint* item = this->getSketchObjectPtr()->Constraints[Index];
-    if (item->Name != Name) {
-        Constraint* copy = item->clone();
-        copy->Name = Name;
-        this->getSketchObjectPtr()->Constraints.set1Value(Index, copy);
-        delete copy;
-    }
+    this->getSketchObjectPtr()->renameConstraint(Index, Name);
+
     Py_Return;
 }