Qt5 crash on close

Question

My project crashes on close sometimes, but not reliably. When closed with the "Cancel" button, it crashes much less often than if closed with the escape key, which just further confuses me. I've got a simple find dialog (a small part of a larger project) as a form of a test case.

Attempts to debug with GDB result in it never crashing. Core dump is garbage (bad memory).

Included below is the source. I haven't been able to pinpoint what's actually going wrong; I've seen (with minor modifications) double free, free-ing NULL, free-ing a pointer that never existed, etc. and I'm beginning to wonder if it's partially Qt's fault.

Header:

#ifndef NIDE_FINDDIALOG_HPP
#define NIDE_FINDDIALOG_HPP

#include <QDialog>
#include <QString>
#include <QGridLayout>
#include <QCheckBox>
#include <QLineEdit>
#include <QPushButton>
#include <QWidget>

class FindDialog: public QDialog {
public:
    bool regex, caseSensitive, inSelection, wholeWord, forward, wrap;
    QString expr;

private:
    struct {
        QGridLayout *layout;
        QCheckBox *regex, *caseSense, *select, *whole, *forward, *wrap;
        QLineEdit *exprBox;
        QPushButton *cancel, *find;
    }ui;

    void onCancel();
    void onFind();

    void onRegex();
    void onCase();
    void onSelect();
    void onWhole();
    void onForward();
    void onWrap();

public:
    FindDialog(QWidget *parent);
    ~FindDialog();
};

#endif/*NIDE_FINDDIALOG_HPP*/

Source:

#include <NIDE/FindDialog.hpp>

FindDialog::FindDialog(QWidget *parent): QDialog(parent),
    regex(false), caseSensitive(true), inSelection(false), wholeWord(true),
    forward(true), wrap(true), expr() {

    ui.layout       = new QGridLayout(this);    
    ui.regex        = new QCheckBox("Regex", this);
    ui.caseSense    = new QCheckBox("Match case", this);
    ui.select       = new QCheckBox("In Selection", this);
    ui.whole        = new QCheckBox("Whole Word", this);
    ui.forward      = new QCheckBox("Forward", this);
    ui.wrap         = new QCheckBox("Wrap around", this);
    ui.exprBox      = new QLineEdit(this);
    ui.cancel       = new QPushButton("Cancel", this);
    ui.find         = new QPushButton("Find", this);

    ui.regex->setChecked(regex);
    connect(ui.regex, &QCheckBox::stateChanged, this, &FindDialog::onRegex);

    ui.caseSense->setChecked(caseSensitive);
    connect(ui.caseSense, &QCheckBox::stateChanged, this, &FindDialog::onCase);

    ui.select->setChecked(inSelection);
    connect(ui.select, &QCheckBox::stateChanged, this, &FindDialog::onSelect);

    ui.whole->setChecked(wholeWord);
    connect(ui.whole, &QCheckBox::stateChanged, this, &FindDialog::onWhole);

    ui.forward->setChecked(forward);
    connect(ui.forward, &QCheckBox::stateChanged, this, &FindDialog::onForward);

    ui.wrap->setChecked(wrap);
    connect(ui.wrap, &QCheckBox::stateChanged, this, &FindDialog::onWrap);

    ui.exprBox->setPlaceholderText(tr("Find expr..."));
    connect(ui.exprBox, &QLineEdit::returnPressed, this, &FindDialog::onFind);

    connect(ui.cancel, &QPushButton::clicked, this, &FindDialog::onCancel);

    ui.find->setDefault(true);
    connect(ui.find, &QPushButton::clicked, this, &FindDialog::onFind);


    ui.layout->addWidget(ui.regex, 0, 0);
    ui.layout->addWidget(ui.caseSense, 0, 1);
    ui.layout->addWidget(ui.select, 0, 2);
    ui.layout->addWidget(ui.whole, 1, 0);
    ui.layout->addWidget(ui.forward, 1, 1);
    ui.layout->addWidget(ui.wrap, 1, 2);
    ui.layout->addWidget(ui.exprBox, 2, 0, 1, 3);
    ui.layout->addWidget(ui.cancel, 3, 1);
    ui.layout->addWidget(ui.find, 3, 2);

    setLayout(ui.layout);

    setWindowTitle(tr("Find"));
}
FindDialog::~FindDialog() {
    delete ui.layout;
    delete ui.regex;
    delete ui.caseSense;
    delete ui.select;
    delete ui.whole;
    delete ui.forward;
    delete ui.wrap;
    delete ui.exprBox;
    delete ui.cancel;
    delete ui.find;
}

void FindDialog::onCancel() {
    done(QDialog::Rejected);
}
void FindDialog::onFind() {
    expr = ui.exprBox->text();
    done(QDialog::Accepted);
}

void FindDialog::onRegex() { regex = ui.regex->isChecked(); }
void FindDialog::onCase() { caseSensitive = ui.caseSense->isChecked(); }
void FindDialog::onSelect() { inSelection = ui.select->isChecked(); }
void FindDialog::onWhole() { wholeWord = ui.whole->isChecked(); }
void FindDialog::onForward() { forward = ui.forward->isChecked(); }
void FindDialog::onWrap() { wrap = ui.wrap->isChecked(); }

Main:

#include <NIDE/FindDialog.hpp>

#include <QApplication>

int main(int argc, char **argv) {
    QApplication *app = new QApplication(argc, argv);
    FindDialog fd(NULL);

    app->setApplicationName("NIDE");

    fd.exec();
}

Answer 1

In avoiding the use of Q_OBJECT macro you're relying on an implementation detail of Qt. It is undefined behavior to connect to methods that were declared in a class without Q_OBJECT. The fact that it works in a particular version of Qt is a happy coincidence.

Alas, your code does a lot of unnecessary things.

1. There's no need to keep Ui elements or QApplication instance on the heap.

2. There's no reason for all of the onXxxx methods since you won't be reading the state of the dialog while it is being shown through exec(). You only care about the state of the dialog after it has been accepted (or perhaps rejected).

3. You can leverage QDialog::accept and QDialog::reject slots.

4. To make the dialog look correctly on multiple platforms, you should use a QDialogButtonBox instead of discrete buttons.

5. Finally, using QDialog::exec() reenters the event loop and makes you think that your code is synchronous when it really isn't. It's a source of hard to find bugs. You should simply show() the dialog and react to the accepted signal that it emits when the Find button is clicked.

The code below attempts at being a reasonably correct solution. It works in both Qt 4 and Qt 5.

// Interface
#include <QDialog>
#include <QGridLayout>
#include <QCheckBox>
#include <QLineEdit>
#include <QDialogButtonBox>

#if QT_VERSION<QT_VERSION_CHECK(5,0,0)
#define Q_DECL_OVERRIDE
#endif

class FindDialog: public QDialog {
   struct Ui {
      QGridLayout layout;
      QCheckBox regex, caseSense, select, whole, forward, wrap;
      QLineEdit exprBox;
      QDialogButtonBox buttonBox;
      Ui(QWidget * widget);
   } m_ui;
   void get();
public:
   bool regex, caseSensitive, inSelection, wholeWord, forward, wrap;
   QString expr;

   FindDialog(QWidget *parent = 0);
   ~FindDialog();
   void set();
   void done(int r) Q_DECL_OVERRIDE;
};

// Implementation
FindDialog::Ui::Ui(QWidget * widget) :
   layout(widget),
   regex(tr("Regex")),
   caseSense(tr("Match case")),
   select(tr("In Selection")),
   whole(tr("Whole Word")),
   forward(tr("Forward")),
   wrap(tr("Wrap around")),
   buttonBox(QDialogButtonBox::Cancel)
{
   layout.addWidget(&regex, 0, 0);
   layout.addWidget(&caseSense, 0, 1);
   layout.addWidget(&select, 0, 2);
   layout.addWidget(&whole, 1, 0);
   layout.addWidget(&forward, 1, 1);
   layout.addWidget(&wrap, 1, 2);
   layout.addWidget(&exprBox, 2, 0, 1, 3);
   layout.addWidget(&buttonBox, 3, 0, 1, 3);
   exprBox.setPlaceholderText(tr("Find expr..."));
   buttonBox.addButton(tr("Find"), QDialogButtonBox::AcceptRole);
}

FindDialog::FindDialog(QWidget *parent): QDialog(parent), m_ui(this),
   regex(false), caseSensitive(true), inSelection(false), wholeWord(true),
   forward(true), wrap(true)
{
   set();
   connect(&m_ui.buttonBox, SIGNAL(rejected()), SLOT(reject()));
   connect(&m_ui.buttonBox, SIGNAL(accepted()), SLOT(accept()));
   setWindowTitle("Find");
}

FindDialog::~FindDialog() {}

void FindDialog::done(int result)
{
   get();
   QDialog::done(result);
}

void FindDialog::get()
{
   regex = m_ui.regex.isChecked();
   caseSensitive = m_ui.caseSense.isChecked();
   inSelection = m_ui.select.isChecked();
   wholeWord = m_ui.whole.isChecked();
   forward = m_ui.forward.isChecked();
   wrap = m_ui.wrap.isChecked();
   expr = m_ui.exprBox.text();
}

void FindDialog::set()
{
   m_ui.regex.setChecked(regex);
   m_ui.caseSense.setChecked(caseSensitive);
   m_ui.select.setChecked(inSelection);
   m_ui.whole.setChecked(wholeWord);
   m_ui.forward.setChecked(forward);
   m_ui.wrap.setChecked(wrap);
}

// Main
#include <QApplication>
#include <QMessageBox>

int main(int argc, char **argv) {
   QApplication app(argc, argv);
   app.setApplicationName("NIDE");
   FindDialog fd;
   fd.show();
#if QT_VERSION>=QT_VERSION_CHECK(5,0,0)
   // This can't be done in Qt4 without using moc
   QObject::connect(&fd, &QDialog::accepted, [&fd]{
      QMessageBox::information(NULL, "Find",
         QString("The user wants to find \"%1\"").arg(fd.expr));
   });
#endif
   return app.exec();
}