search

esternin / eXtrema

https://www.physics.brocku.ca/Labs/extrema/
GNU General Public License v2.0
6 stars 1 forks source link

Fix memory leaks when showing dialogs #9

Closed vadz closed 3 years ago

vadz commented 3 years ago

There are plenty (>200) places doing something like this:

    wxMessageDialog *md = new wxMessageDialog( this, wxString(e.what(),wxConvUTF8),
                                               wxT("Error"), wxOK|wxICON_ERROR );
    md->ShowModal();

This is a memory leak as modal dialogs are not destroyed by wxWidgets (this is the only exception to the general rule) and so each and every call like this leaks memory and a window. We need to fix this as after a sufficient number of errors (but this is also done for modal file selection dialogs), the program is sure to run out of resources/windows and crash.

vadz commented 3 years ago

OK, so this took a bunch of Vim substitutions to do, but should be fixed now.

Review of the commit mentioned above would be welcome, as well as your testing, of course -- even though it's not supposed to change anything and I've tried to review all the changes, most of them have been done semi-automatically.

esternin commented 3 years ago

Not sure if this is related, but there is now a crash after the first PAUSE.

This is the old-style text-mode equivalent of inquire\yesno.

For example, the old demo/demo.pcm seg faults after the first pause. (This is not the new Scripts/demo.pcm)

esternin commented 3 years ago

Not sure how to test for modal/memory leaks, but the dialogs seem to behave normally, so as expected, the fix did not really change anything, except made the possible crashes less frequent?

The PAUSE crash was a separate issue, already closed. Closing this.

vadz commented 3 years ago

Testing for leaks can be done either by building with ASAN (address sanitizer) or using valgrind, but there are currently too many reports from ASAN to be useful, so I've just fixed this one because it was so egregious and repeated everywhere (probably by copy-pasting the original mistake around, which is why I wanted to stop it from propagating further).