4

I developed a Qt application that has an authentication widget and a main window. After authentication succeed I open the mainWindow using this code

this->~Authentification();
MainWindow *w = new MainWindow(); 

but after I close the main window I receive a double free or corruption error. the debug said that the source of this error is

delete ui;

from the class Authentification

demonplus
  • 5,613
  • 12
  • 49
  • 68
Dababi
  • 185
  • 2
  • 17
  • Does Authentification inehrit from QObject by any chance? Also invoking destructors from within a class while technically possible is tricky to debug if somethnig goes wrong (you must guarantee it is not touched later). If Authentification IS QObject then I suggest using this->deleteLater(); Plus you don't pass MainWindow a parent and since you are calling new in now destroyed class you leak memory as you cannot possibly obtain that pointer to delete it. If you do show it after that call and then delete it... you are still operating in destroyed class. Looks like very bad design decision. – Resurrection Jul 12 '16 at 09:26
  • NEVER call destructors explicitly. Except "placement new" that is a rare case. – ilotXXI Jul 12 '16 at 09:27
  • yes Authentification is a QWidget and inehrit from QObject. the problem is when i called this->~Authentification();. the authentification class isn't completely closed and when i close the mainwindow it also calls ~Authentification() another time. – Dababi Jul 12 '16 at 09:32
  • @Dababi Never call delete on QObjects and never ever ever call their destructor directly. And oh god why do you do that from within itself?! I mean Qt has super fine tuned and easy memory management system so why do you even attempt this? If you must, at least use deleteLater(). Plus you can wire (connect) MainWindow to its deleteLater slot on its close signal if there is no suitable parent around or you cannot make it on the stack. – Resurrection Jul 12 '16 at 09:34
  • thanks @ Resurrection, when i remove this->~Authentification(); the problem disappear but i need to close the authentification window . whatis the best way to do this. – Dababi Jul 12 '16 at 09:40
  • "Never call delete on QObjects" - there are plenty of cases where deleting QObjects explicitly is perfectly fine. For example all cases where three is no parent object or where the parent survives the child object – Frank Osterfeld Jul 12 '16 at 09:58
  • Could you please show how you initialize Authentication? – demonplus Jul 12 '16 at 10:03
  • thanks, I used the close() function and it worked very well – Dababi Jul 12 '16 at 10:04
  • @FrankOsterfeld You are wrong but I will not repeat documentation nor other questions, e.g. http://stackoverflow.com/questions/4888189/how-delete-and-deletelater-works-with-regards-to-signals-and-slots-in-qt The fact you can get away with it most of the time does not make it a "pefectly fine" thing to do. Unlike signals and slots that are disconnected on QObject destruction events waiting in the event loop are still dispatched to (now deleted) the object and that is UB. – Resurrection Jul 12 '16 at 10:10
  • @Resurrection: In the question you linked, my answer gives two cases where direct delete doesn't work, plus the multi-threaded event handling case mentioned by liaK. Where those scenarios do not apply, delete works just fine. – Frank Osterfeld Jul 12 '16 at 12:04
  • @FrankOsterfeld And what about answer by Piotr Dobrogost? Particularly the bit that "it works now but may not work later" is one of the nastier things in debugging. Finding out the code that worked earlier now crashes your app because you changed something seemingly unrelated is a nightmare. You still insist it is ok to use plain delete even though you cannot guarantee there are no asynchronous events in the queue or (more importantly) you won't accidentally change something indirectly related to it in the future only to break it. So why take that risk? – Resurrection Jul 12 '16 at 12:15

1 Answers1

2

You're explicitly invoking the destructor on this. There are very few times you'd need to do this, and they always should be abstracted out. Such calls only belong in low-level resource management classes. If you think of doing this in a subclass of QObject or QWidget, you most likely shouldn't be!

If all you want is to close a window, use QWidget::close(). But perhaps you wish to destroy the widget instance, too, to release any resources used by it. Then read on.

Let's assume that Authentication is a proper dialog that emits accepted() and rejected() signals as appropriate when authentication succeeds or fails, respectively:

class Authentication : public QDialog {
  ...
};

Some ways of proceeding from such a dialog might be:

  1. Define the dialog as a variable local to a scope, run an event loop as long as the dialog is active, then leave the scope:

    int main(int argc, char ** argv) {
      QApplication app{argc, argv};
      {
        Authentication auth;
        auto result = auth.exec();
        if (result == QDialog::Rejected) return 1;
      } // here auth has been destructed
      MainWindow window;
      window.show();
      return app.exec();
    }  
    
  2. Allocate the dialog dynamically, and have it auto-delete when closed.

    int main(int argc, char ** argv) {
      QApplication app{argc, argv};
      auto auth = new Authentication;
      auth->setAttribute(Qt::WA_DeleteOnClose);
      QObject::connect(auth, &QDialog::accepted, []{
        auto win = new MainWindow;
        win->setAttribute(Qt::WA_DeleteOnClose);
        win->show();
      });
      auth->show();
      return app.exec();
    }
    
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313