Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. QGraphicsScene onSelectionChanged crashes application on close. BUG?

QGraphicsScene onSelectionChanged crashes application on close. BUG?

Scheduled Pinned Locked Moved Solved General and Desktop
c++ qtbugqgraphicsscenecrash
12 Posts 4 Posters 952 Views
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • S Offline
    S Offline
    StudentScripter
    wrote on last edited by StudentScripter
    #1

    So i connect my QGraphicsScene subclass to the selection changed signal and created a slot for it. However now everytime i close my application it crashes/gives an error:

    ASSERT failure in CustomGraphicsScene: "Called object is not of the correct type (class destructor may have already run)", file C:/Users/Musik/Documents/Qt/6.5.2/mingw_64/include/QtCore/qobjectdefs_impl.h, line 119
    

    Here is the minimal code example:

    
    CustomGraphicsScene::CustomGraphicsScene(QObject *parent)
        : QGraphicsScene(parent)
    {   
    undoStack = new QUndoStack(this);
    
        
        //Setzt die generelle Hintergrundfarbe für die gesamte Scene
        setBackgroundBrush(Qt::gray);
      
        //Höhe & Breite, setzt das eigentliche SceneRect
        SceneRectWidth = 900;
        SceneRectHeight = 600;
        setSceneRect(QRectF(0, 0, SceneRectWidth, SceneRectHeight));
        
    
            
    
    
            // Connect the selectionChanged signal to the handleSelectionChange slot
            if (!connect(this, &QGraphicsScene::selectionChanged, this, &CustomGraphicsScene::onSelectionChanged)) {
                qDebug() << "Failed to connect signal and slot";
            }
    
            
    }
    
    
    CustomGraphicsScene::~CustomGraphicsScene()
    {
    
        qDebug() << "isDestructed";
    }
    
    
    
    
    
    void CustomGraphicsScene::onSelectionChanged()
    {qDebug() << "Test";
        if(isUpdatingSelection == false){
        QList<QGraphicsItem*> selectedItems = this->selectedItems();
        this->update();
    
        emit SendSelectionChangedFromSceneToLayerList(selectedItems);
        }
    }
    

    The only solution to fix this is manually disconnecting the signal in the destructor of the graphicsscene. But as far as i know this shouldn't be the standard/normal behaviour with signal and slots:

    This fixes the problem:

    CustomGraphicsScene::~CustomGraphicsScene()
    {
        disconnect(this, &QGraphicsScene::selectionChanged, this, &CustomGraphicsScene::onSelectionChanged);
    
        qDebug() << "isDestructed";
    }
    
    
    

    Or what also does work: using the old signal and slots syntax:

            if (!connect(this, SIGNAL(selectionChanged()), this, SLOT(onSelectionChanged()))) {
                qDebug() << "Failed to connect signal and slot";
            }
    

    Any ideas why this is happening? Am i doing something wrong?

    JoeCFDJ 1 Reply Last reply
    1
    • JoeCFDJ JoeCFD

      @StudentScripter it is not something bad to write disconnection in the destructor. Actually, I do it often. It is not that you must do it. However, sometimes things can happen like
      the signal is sent out, but the receiver is then destroyed like in your case.
      Your app will crash.

      S Offline
      S Offline
      StudentScripter
      wrote on last edited by
      #3

      @JoeCFD Ok thanks,
      i thought this was some kind of odd behaviour as i've read somewhere that qt manages this out of the box (and with the old syntax it works without disconneting)

      Chris KawaC 1 Reply Last reply
      0
      • S StudentScripter

        So i connect my QGraphicsScene subclass to the selection changed signal and created a slot for it. However now everytime i close my application it crashes/gives an error:

        ASSERT failure in CustomGraphicsScene: "Called object is not of the correct type (class destructor may have already run)", file C:/Users/Musik/Documents/Qt/6.5.2/mingw_64/include/QtCore/qobjectdefs_impl.h, line 119
        

        Here is the minimal code example:

        
        CustomGraphicsScene::CustomGraphicsScene(QObject *parent)
            : QGraphicsScene(parent)
        {   
        undoStack = new QUndoStack(this);
        
            
            //Setzt die generelle Hintergrundfarbe für die gesamte Scene
            setBackgroundBrush(Qt::gray);
          
            //Höhe & Breite, setzt das eigentliche SceneRect
            SceneRectWidth = 900;
            SceneRectHeight = 600;
            setSceneRect(QRectF(0, 0, SceneRectWidth, SceneRectHeight));
            
        
                
        
        
                // Connect the selectionChanged signal to the handleSelectionChange slot
                if (!connect(this, &QGraphicsScene::selectionChanged, this, &CustomGraphicsScene::onSelectionChanged)) {
                    qDebug() << "Failed to connect signal and slot";
                }
        
                
        }
        
        
        CustomGraphicsScene::~CustomGraphicsScene()
        {
        
            qDebug() << "isDestructed";
        }
        
        
        
        
        
        void CustomGraphicsScene::onSelectionChanged()
        {qDebug() << "Test";
            if(isUpdatingSelection == false){
            QList<QGraphicsItem*> selectedItems = this->selectedItems();
            this->update();
        
            emit SendSelectionChangedFromSceneToLayerList(selectedItems);
            }
        }
        

        The only solution to fix this is manually disconnecting the signal in the destructor of the graphicsscene. But as far as i know this shouldn't be the standard/normal behaviour with signal and slots:

        This fixes the problem:

        CustomGraphicsScene::~CustomGraphicsScene()
        {
            disconnect(this, &QGraphicsScene::selectionChanged, this, &CustomGraphicsScene::onSelectionChanged);
        
            qDebug() << "isDestructed";
        }
        
        
        

        Or what also does work: using the old signal and slots syntax:

                if (!connect(this, SIGNAL(selectionChanged()), this, SLOT(onSelectionChanged()))) {
                    qDebug() << "Failed to connect signal and slot";
                }
        

        Any ideas why this is happening? Am i doing something wrong?

        JoeCFDJ Offline
        JoeCFDJ Offline
        JoeCFD
        wrote on last edited by
        #2

        @StudentScripter it is not something bad to write disconnection in the destructor. Actually, I do it often. It is not that you must do it. However, sometimes things can happen like
        the signal is sent out, but the receiver is then destroyed like in your case.
        Your app will crash.

        S 1 Reply Last reply
        0
        • JoeCFDJ JoeCFD

          @StudentScripter it is not something bad to write disconnection in the destructor. Actually, I do it often. It is not that you must do it. However, sometimes things can happen like
          the signal is sent out, but the receiver is then destroyed like in your case.
          Your app will crash.

          S Offline
          S Offline
          StudentScripter
          wrote on last edited by
          #3

          @JoeCFD Ok thanks,
          i thought this was some kind of odd behaviour as i've read somewhere that qt manages this out of the box (and with the old syntax it works without disconneting)

          Chris KawaC 1 Reply Last reply
          0
          • S StudentScripter

            @JoeCFD Ok thanks,
            i thought this was some kind of odd behaviour as i've read somewhere that qt manages this out of the box (and with the old syntax it works without disconneting)

            Chris KawaC Offline
            Chris KawaC Offline
            Chris Kawa
            Lifetime Qt Champion
            wrote on last edited by Chris Kawa
            #4

            @StudentScripter said:

            i've read somewhere that qt manages this out of the box

            It does in general, but you've hit a very niche corner case of polymorphic destruction. The order of destruction for your class is the reverse order of construction, so:

            ~CustomGraphicsScene()
            ~QGraphicsScene()
            ~QObject()
            

            The automatic disconnection is handled in the QObject destructor, so last in this case. Before it happens and connection is still alive a QGraphicsScene destructor is called, which apparently signals selectionChanged, probably unselecting stuff before destruction. The problem is it is connected to onSelectionChanged, which is a method of CustomGraphicsScene. The destructor of CustomGraphicsScene was already invoked at that time, so CustomGraphicsScene part of the object is not valid and calling this->update() on it is undefined behavior. In this case it crashes.

            The disconnection in CustomGraphicsScene destructor fixes the issue because the entire object is still alive at that point, so you can refer to it and do the disconnection.

            The macro based connection also works because it does string based lookup at runtime, and since CustomGraphicsScene part doesn't exist anymore at the signal time, the lookup silently fails and doesn't invoke the slot. Pointer based connection is more like a direct call on a pointer, so it goes through whether the object is valid or not.

            JonBJ 1 Reply Last reply
            5
            • Chris KawaC Chris Kawa

              @StudentScripter said:

              i've read somewhere that qt manages this out of the box

              It does in general, but you've hit a very niche corner case of polymorphic destruction. The order of destruction for your class is the reverse order of construction, so:

              ~CustomGraphicsScene()
              ~QGraphicsScene()
              ~QObject()
              

              The automatic disconnection is handled in the QObject destructor, so last in this case. Before it happens and connection is still alive a QGraphicsScene destructor is called, which apparently signals selectionChanged, probably unselecting stuff before destruction. The problem is it is connected to onSelectionChanged, which is a method of CustomGraphicsScene. The destructor of CustomGraphicsScene was already invoked at that time, so CustomGraphicsScene part of the object is not valid and calling this->update() on it is undefined behavior. In this case it crashes.

              The disconnection in CustomGraphicsScene destructor fixes the issue because the entire object is still alive at that point, so you can refer to it and do the disconnection.

              The macro based connection also works because it does string based lookup at runtime, and since CustomGraphicsScene part doesn't exist anymore at the signal time, the lookup silently fails and doesn't invoke the slot. Pointer based connection is more like a direct call on a pointer, so it goes through whether the object is valid or not.

              JonBJ Offline
              JonBJ Offline
              JonB
              wrote on last edited by
              #5

              @Chris-Kawa
              In view of all of this, perhaps @StudentScripter should do the disconnect() for all slots, in case there are others (maybe in the future)?

              JoeCFDJ 1 Reply Last reply
              0
              • JonBJ JonB

                @Chris-Kawa
                In view of all of this, perhaps @StudentScripter should do the disconnect() for all slots, in case there are others (maybe in the future)?

                JoeCFDJ Offline
                JoeCFDJ Offline
                JoeCFD
                wrote on last edited by JoeCFD
                #6

                @JonB Maybe he can also try to call CustomGraphicsScene->deleteLater() without disconnection. We do clean-up our connections in the destructors in a heavily multi-threaded app.

                Chris KawaC 1 Reply Last reply
                0
                • JoeCFDJ JoeCFD

                  @JonB Maybe he can also try to call CustomGraphicsScene->deleteLater() without disconnection. We do clean-up our connections in the destructors in a heavily multi-threaded app.

                  Chris KawaC Offline
                  Chris KawaC Offline
                  Chris Kawa
                  Lifetime Qt Champion
                  wrote on last edited by
                  #7

                  @JoeCFD said:

                  Maybe he can also try to call CustomGraphicsScene->deleteLater() without disconnection

                  I don't see how this would change anything. deleteLater just postpones the deletion. The problem is still in the destructor, whenever it is called.

                  The easiest is, as the OP did - do the disconnection in the destructor. You could also do like @JonB said, call disconnect() in the destructor to break all connections to this object. You do risk disconnecting something you might not want to though, like somebody connected to QObject::destroyed signal, so it's usually not a good idea to do a wide sweep like that.

                  If that's the only connection causing trouble in that class you can also be more subtle and just call clearSelection() in the CustomGraphicsScene destructor, so that the signal won't trigger down the destructor stack, avoiding the whole kerfuffle altogether.

                  All in all it's a very specific corner case. It's similar to something like this:

                  class Base {
                     ~Base() { ((Derived*)this)->something(); }
                  }
                  

                  just a bit more covered up. It's not something you do on purpose.

                  JonBJ 1 Reply Last reply
                  0
                  • Chris KawaC Chris Kawa

                    @JoeCFD said:

                    Maybe he can also try to call CustomGraphicsScene->deleteLater() without disconnection

                    I don't see how this would change anything. deleteLater just postpones the deletion. The problem is still in the destructor, whenever it is called.

                    The easiest is, as the OP did - do the disconnection in the destructor. You could also do like @JonB said, call disconnect() in the destructor to break all connections to this object. You do risk disconnecting something you might not want to though, like somebody connected to QObject::destroyed signal, so it's usually not a good idea to do a wide sweep like that.

                    If that's the only connection causing trouble in that class you can also be more subtle and just call clearSelection() in the CustomGraphicsScene destructor, so that the signal won't trigger down the destructor stack, avoiding the whole kerfuffle altogether.

                    All in all it's a very specific corner case. It's similar to something like this:

                    class Base {
                       ~Base() { ((Derived*)this)->something(); }
                    }
                    

                    just a bit more covered up. It's not something you do on purpose.

                    JonBJ Offline
                    JonBJ Offline
                    JonB
                    wrote on last edited by
                    #8

                    @Chris-Kawa said in QGraphicsScene onSelectionChanged crashes application on close. BUG?:

                    like somebody connected to QObject::destroyed signal, so it's usually not a good idea to do a wide sweep like that.

                    OOhh, good one, I didn't think of that! I meant all the ones other than destroyed() ;-)

                    S Chris KawaC 2 Replies Last reply
                    0
                    • JonBJ JonB

                      @Chris-Kawa said in QGraphicsScene onSelectionChanged crashes application on close. BUG?:

                      like somebody connected to QObject::destroyed signal, so it's usually not a good idea to do a wide sweep like that.

                      OOhh, good one, I didn't think of that! I meant all the ones other than destroyed() ;-)

                      S Offline
                      S Offline
                      StudentScripter
                      wrote on last edited by
                      #9

                      @JonB @Chris-Kawa Very good insights on this one. Definitely learned a new thing here. Thanks you all, i'll closing this post as my concerns have been adressed. Have a nice day. :D

                      1 Reply Last reply
                      0
                      • S StudentScripter has marked this topic as solved on
                      • JonBJ JonB

                        @Chris-Kawa said in QGraphicsScene onSelectionChanged crashes application on close. BUG?:

                        like somebody connected to QObject::destroyed signal, so it's usually not a good idea to do a wide sweep like that.

                        OOhh, good one, I didn't think of that! I meant all the ones other than destroyed() ;-)

                        Chris KawaC Offline
                        Chris KawaC Offline
                        Chris Kawa
                        Lifetime Qt Champion
                        wrote on last edited by
                        #10

                        @JonB said:

                        I meant all the ones other than destroyed()

                        Well, destroyed() is probably not that common, but imagine a QAction somewhere that is enabled/disabled on selection change. That's something you see often. Disconnecting it early means it wouldn't get that last unselect signal and could be left enabled when it shouldn't be and crash when triggered. You could be trading one crash for another one somewhere else.

                        That's why I usually avoid any sweeping actions. You never know what gets caught in them that shouldn't.

                        JonBJ 1 Reply Last reply
                        0
                        • Chris KawaC Chris Kawa

                          @JonB said:

                          I meant all the ones other than destroyed()

                          Well, destroyed() is probably not that common, but imagine a QAction somewhere that is enabled/disabled on selection change. That's something you see often. Disconnecting it early means it wouldn't get that last unselect signal and could be left enabled when it shouldn't be and crash when triggered. You could be trading one crash for another one somewhere else.

                          That's why I usually avoid any sweeping actions. You never know what gets caught in them that shouldn't.

                          JonBJ Offline
                          JonBJ Offline
                          JonB
                          wrote on last edited by
                          #11

                          @Chris-Kawa
                          Well, since you have brought this up (and the OP is satisfied with the answers in this thread). Plenty of posts here and on SO are quite happy to recommend calling blockSignals() in situations where people don't want signals delivered. Which I have always regraded as devil's spawn for precisely the reasons you are mentioning, you have no idea what you are blocking and what the consequences might be....

                          Chris KawaC 1 Reply Last reply
                          1
                          • JonBJ JonB

                            @Chris-Kawa
                            Well, since you have brought this up (and the OP is satisfied with the answers in this thread). Plenty of posts here and on SO are quite happy to recommend calling blockSignals() in situations where people don't want signals delivered. Which I have always regraded as devil's spawn for precisely the reasons you are mentioning, you have no idea what you are blocking and what the consequences might be....

                            Chris KawaC Offline
                            Chris KawaC Offline
                            Chris Kawa
                            Lifetime Qt Champion
                            wrote on last edited by
                            #12

                            @JonB said:

                            Which I have always regraded as devil's spawn

                            Agreed. I see blockSignals() as an advanced tool to be used with great care and only when you're very aware of all consequences in given case. Similar to sender(). Definitely not something to be applied on a "let's see if it helps" basis.

                            1 Reply Last reply
                            0

                            • Login

                            • Login or register to search.
                            • First post
                              Last post
                            0
                            • Categories
                            • Recent
                            • Tags
                            • Popular
                            • Users
                            • Groups
                            • Search
                            • Get Qt Extensions
                            • Unsolved