Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-60859

Memory leak in "Undo Framework Example": add / remove item

    XMLWordPrintable

Details

    Description

      QT undo framework example has a bug with the way that it calls the destructor of the items.

      QGraphicsScene assume ownership of the items while they are in the scene, however both undo objects: AddCommand and RemoveCommand should assume ownership when they remove them from the scene.

      In the Qt undo framework example, only AddCommand tries to delete the object in its destructor, but will not do it if the item is still in the scene.

      AddCommand::~AddCommand() {
          if (!myDiagramItem->scene()) delete myDiagramItem;
       }
      

      If we remove the item from the scene after AddCommand object leave the stack (when using an undo limit), the item will never be deleted again since RemoveCommand destructor doesn't do it, resulting in a memory leak.

      In other words, when setting an undo limit, the AddCommand destructor may be called with the item still on the scene, and in this case the item will never be deleted, even after calling RemoveCommand.

      I fixed it using a flag in both AddCommand and RemoveCommand classes. It defines who should be responsible for the item's destruction. When they remove the item from the scene, I set this flag to true, and I test this flag in its destructor before calling the item's destructor:
       

      AddCommand::AddCommand(QGraphicsScene *scene, DraftGraphicItem* item, QUndoCommand *parent):
          scene(scene), item(item), QUndoCommand(parent), destroyObjResponsability(false){
          setText("Add item to scene");
      }
      
      AddCommand::~AddCommand(){
          if(destroyObjResponsability)
              delete item;
      }
      
      void AddCommand::undo(){
          Q_ASSERT(item->scene()); 
          scene->removeItem(item);
          destroyObjResponsability = true;
      }
      
      void AddCommand::redo(){
          Q_ASSERT(!item->scene()); 
          scene->addItem(item);
          destroyObjResponsability = false;
      }
      

      and the same with RemoveCommand, just inverting redo() and undo() methods.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            docteam Qt Documentation Team
            ahi Adriel Jr.
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:

              Gerrit Reviews

                There are no open Gerrit changes