Memory access errors from QMessageBox

Helson

Try change QMessageBox* messageBox = new QMessageBox;
to
QMessageBox messageBox;
and remove delete messageBox;

raven-worx

first of all you should start the main event-loop (QApplication::exec())
And open the the messageboxes once it runs, then check again if the error occurs.

NigelMcFarlane

@Helson Thank you for your reply. Unfortuntately changing QMessageBox to an automatic variable makes no difference.

NigelMcFarlane

@raven-worx
thank you for your reply. I did forget to add app.exec() at the end, but adding it does not remove the errors.

It's a bit tricky in this small example to run app.exec() before showing the box. However, in my much larger application, the dialogs and message boxes are created by events from a QMainWindow, when the QApplication is already running, and they do suffer from the same problem. Curiously the main window is itself completely clean of errors.

raven-worx

@NigelMcFarlane
the callstacks you've posted don't make much sense and i would say they can be carelessly discarded.
So maybe you are overwriting the some memory in your application .. i don't know.
And obviously you haven't even tried the snipped you've posted but claim that it isn't working. You should run this minimal example and check again if it still crashes. This is as easy as subclassing a plain QWidget and open the dialog on a button press for example.
If it doesn't crash the issue is somewhere else in your code.

NigelMcFarlane

@raven-worx
I may not have attempted your suggestion on the mini-snippet but I most certainly tested the snippet itself before I posted it!!
In any case, I have now found the cause of the crash in my application, and it was not caused by these "leaks". Whatever they are, they seem to be harmless. If no one else is seeing them perhaps they are just artefacts of the diagnostic tool.

mjsurette

Normal practice is to construct with an argument

QMessageBox* messageBox = new QMessageBox(this);

and let Qt automatically delete it when "this" goes out of scope. This follows the RAII principle.

kshegunov

Hello,
Are you sure that this stack trace corresponds to the particular example code?
These two lines:

#18 Qt5Cored.dll!QSortFilterProxyModel::mapToSource +0x2ca891 (0x66a26b47 <Qt5Cored.dll+0x2e6b47>)
#19 Qt5Cored.dll!QSortFilterProxyModel::mapToSource +0x3e9a42 (0x66b45cf8 <Qt5Cored.dll+0x405cf8>)

are most certainly very suspicious. I wouldn't expect a message box to have anything to do with any of the model classes. That said, I don't believe you can have a modal dialog as a main window, it makes no sense - it should be modal to what? Maybe simply try showing it as you would any other ordinary widget, i.e:

int main(int argc, char ** argv)
{
    QApplication app(argc, argv); // application object manages everything

    QMessageBox messageBox;
    messageBox.setWindowTitle("My Message");
    messageBox.setText("Hello World");
    messageBox.show();

    return QApplication::exec();
}

Kind regards.

NigelMcFarlane

@mjsurette
Thank you for your reply, I did not know that Qt objects were smart pointers in this way.
Regards
Nigel

NigelMcFarlane

@kshegunov
Thank you for reply.
I don't think the error list actually represents a stack trace in any way, though I agree that the list of classes involved is strange.

Now I know these "errors" were a red herring in terms of my larger application crashing; they seem to be harmless, possibly an artefact of Dr Memory or my Windows installation, and I guess not of great general interest. If I get a moment I might try running a different memory tool for comparison.
Thank you
Best regards
Nigel

kshegunov

@NigelMcFarlane
Hello,

Thank you for your reply, I did not know that Qt objects were smart pointers in this way.

QObjects are smart in many ways, they can and will make your life easier in multitude of cases, but one should know when and how to use them cleverly. I will argue what @mjsurette suggested, namely this:

Normal practice is to construct with an argument
QMessageBox* messageBox = new QMessageBox(this);
and let Qt automatically delete it when "this" goes out of scope. This follows the RAII principle.

to be a bad advice! It is true that your QWidget (this is supposed to be a QWidget in this case) will free the message box in its destructor, however you're delegating responsibility for a local variable to the global scope (not program-global, but class-global). What happens to the message box when you give it a parent is that it's added to the list of children of your widget. Putting aside the visual implications (as children are shown only inside their parent widget) you're creating an (almost) dangling object - you lose the ability to manipulate and/or delete it as soon as your function goes out of scope. In many cases this isn't a problem for objects that are in actuality expected to persist in memory (i.e. creating a layout for a widget), because naturally the widget takes ownership of the layout and uses it to display itself, unfortunately this is not the case here. Suppose you have that code in a function and that function is called a 100 times, this code leads to 100 QMessageBox objects just waiting in the children list to be deleted some day when the parent has been destroyed. Since most QWidget instances persist through the whole lifetime of your application this simply means that the memory is freed when your application exits. Remember neither Qt nor C++ is Java! Another point is that a QObject is already dragging along a heap allocation for its private object (the PIMPL idiom is used throughout Qt), so there is not really a good reason to use heap allocations for local variables, the stack is faster, more robust and less error-prone. The RAII comment is just plainly wrong, since any local stack variable does not break RAII by design. When the stack is unwinding the local object's destructor will be called and memory freed, it doesn't matter whether the reason was an exception being thrown or the function going out of scope. Since message boxes are mostly modal the best way is to use:

QMessageBox messageBox;
messageBox.setText("Some text");
// ... More initialization

messageBox.exec(); //< This waits for the user input and returns when the message box is closed.
// .. Handle here the return value of `QMessageBox::exec`

Another option is to use one of the pre-made static functions available for the QMessageBox class, which would be my preferred choice. However, as I said in my previous comment, this is a bit different when you use the message box as a top-level window (no parent, defined in main), because you don't have the QApplication's event loop running when show()/exec() is called.

Back to the original issue

The lines I cited do look like a stack trace and don't correspond to the actual code provided, hence @raven-worx's frustration that I share. If I had to guess what the problem is, my suggestion is to check your memory allocation/freeing, as this heap problems occur often on double delete or dereferencing dangling pointer, but no one can say for sure without a code to check against, or at least look upon. If your application is crashing, rely on the debugger to provide the relevant information - where and why this occurred, and not memory inspection tools, they will not give you the answer. If you're instead tracing a (possible) memory leak, then you could try them.

PS. This got a bit lengthy, but I hope is helpful.

Kind regards.

NigelMcFarlane

@kshegunov
Hello,
The report is definitely from the small QMessageBox program. If it's impossible that Qt could be calling these routines from this program, it suggests that the output of Dr Memory is not reliable, at least on my installation.

I set the program to run under VLD and this reported no leaks, which also suggests a problem with Dr Memory.

Regarding the "smart pointers" issue: if I do not delete the QMessageBox, vld reports it as a memory leak.
Best regards
Nigel