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

"Guillermo E. Martinez" <[email protected]>
Newsgroups gmane.linux.umbrello.devel
Message-ID <[email protected]>
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 <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.