[PATCH] Fix: crashing when {un,re}do commands

Guillermo E. Martinez guillermo.e.martinez at oracle.com
Mon Apr 25 12:30:04 BST 2022


Hello umbrello team,

This patch try to fix a SIGSEGV when we're working with undo/redo
commands.

Please let me know your thoughts, I really appreciate them :-)

Best regards,
Guillermo

Executing {un,re}do commands a sigsegv is happing as follow:

    /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
    const*, int) ()
        at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
    (this=0x7fffe4006f10)
        at
    /home/byby/src/umbrello_git/umbrello/cmds/widget/cmdbasewidgetcommand.cpp:77
    (this=0x7fffe4006f10)
        at
    /home/byby/src/umbrello_git/umbrello/cmds/widget/cmdcreatewidget.cpp:124

When `undo' command is triggered, it calls to `o->deleteLater' getting
out of memory the `widget' object, then, doing a `redo' command this
object is not longer available, so an invalid `m_widget' reference is
used.  This patch change this behavior calling `{add,remove}Item' member
function from `{re,un}do' callbacks respectively triggered by
`QUndoStack::push' member.

	* umbrello/cmds/widget/cmdcreatewidget.cpp
	(CmdCreateWidget::CmdCreateWidget): Move `addWidgetToScene' to
	`CmdCreateWidget::redo'.
	(CmdCreateWidget::redo): Use `addWidgetToScene' with `m_widget',
	updating scene object.
	* umbrello/umlscene.cpp (UMLScene::removeWidgetCmd): Remove
	`o->deleteLater' updating the scene.

Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez at oracle.com>
---
 umbrello/cmds/widget/cmdcreatewidget.cpp | 20 +++++++++++---------
 umbrello/umlscene.cpp                    |  2 +-
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/umbrello/cmds/widget/cmdcreatewidget.cpp b/umbrello/cmds/widget/cmdcreatewidget.cpp
index 4ff5db74b..1d78de0d7 100644
--- a/umbrello/cmds/widget/cmdcreatewidget.cpp
+++ b/umbrello/cmds/widget/cmdcreatewidget.cpp
@@ -28,8 +28,6 @@ namespace Uml
     {
         setText(i18n("Create widget : %1", widget->name()));
 
-        addWidgetToScene(widget);
-
         QString xmi;
         QXmlStreamWriter stream(&xmi);
         stream.writeStartElement(QLatin1String("widget"));
@@ -83,16 +81,20 @@ namespace Uml
     void CmdCreateWidget::redo()
     {
         if (!m_isAssoc) {
-            UMLWidget* widget = scene()->findWidget(m_widgetId);
+            UMLWidget* widget = m_widget;
             if (widget == nullptr) {
-                // If the widget is not found, the add command was undone. Load the
-                // widget back from the saved XMI state.
-                QDomElement widgetElement = m_element.firstChild().toElement();
-                widget = scene()->loadWidgetFromXMI(widgetElement);
-                if (widget) {
-                    addWidgetToScene(widget);
+                widget = scene()->findWidget(m_widgetId);
+                if (widget == nullptr) {
+                    // If the widget is not found, the add command was undone. Load the
+                    // widget back from the saved XMI state.
+                    QDomElement widgetElement = m_element.firstChild().toElement();
+                    widget = scene()->loadWidgetFromXMI(widgetElement);
                 }
             }
+            if (widget) {
+                addWidgetToScene(widget);
+                scene()->update();
+            }
         } else {
             AssociationWidget* widget = scene()->findAssocWidget(m_widgetId);
             if (widget)
diff --git a/umbrello/umlscene.cpp b/umbrello/umlscene.cpp
index fbf142698..5a785d556 100644
--- a/umbrello/umlscene.cpp
+++ b/umbrello/umlscene.cpp
@@ -1355,8 +1355,8 @@ void UMLScene::removeWidgetCmd(UMLWidget * o)
     disconnect(this, SIGNAL(sigLineColorChanged(Uml::ID::Type)), o, SLOT(slotLineColorChanged(Uml::ID::Type)));
     disconnect(this, SIGNAL(sigTextColorChanged(Uml::ID::Type)), o, SLOT(slotTextColorChanged(Uml::ID::Type)));
     removeItem(o);
-    o->deleteLater();
     m_doc->setModified(true);
+    update();
 }
 
 /**
-- 
2.35.1



More information about the umbrello-devel mailing list