Crash on program close Topic is solved

[HighCommander4]

Hello all,

I wrote a simple wxWidgets program and it crashes when the program is closed:

Code: Select all

// headers

#include "wx/wx.h"
#include <boost/smart_ptr.hpp>

using boost::shared_ptr;

// IDs for the controls and the menu commands
enum
{
    // menu items
    Minimal_Quit = 1,
    Minimal_About
};

// Define a new frame type: this is going to be our main frame
class MyFrame : public wxFrame
{
public:
    // ctor(s)
    MyFrame(const wxString& title, const wxPoint& pos, const wxSize& size,
            long style = wxDEFAULT_FRAME_STYLE) : wxFrame(NULL, -1, title, pos, size, style),
                                                  fileMenu(new wxMenu),
                                                  helpMenu(new wxMenu),
                                                  menuBar(new wxMenuBar)
    {       
        // the "About" item should be in the help menu
        helpMenu->Append(Minimal_About, "&About...\tF1", "Show about dialog");
    
        fileMenu->Append(Minimal_Quit, "E&xit\tAlt-X", "Quit this program");
    
        // now append the freshly created menu to the menu bar...
        menuBar->Append(fileMenu.get(), "&File");
        menuBar->Append(helpMenu.get(), "&Help");
    
        // ... and attach this menu bar to the frame
        SetMenuBar(menuBar.get());
    
        // create a status bar just for fun (by default with 1 pane only)
        CreateStatusBar();
        SetStatusText("Welcome to wxWindows!");
    }

    // event handlers (these functions should _not_ be virtual)
    void OnQuit(wxCommandEvent& event)
    {
        // TRUE is to force the frame to close
        Close(true);
    }
    void OnAbout(wxCommandEvent& event)
    {
        wxString msg;
        msg.Printf("This is the About dialog of the minimal sample.\nWelcome to %s", wxVERSION_STRING);
    
        wxMessageBox(msg, "About Minimal", wxOK | wxICON_INFORMATION, this);
    }

private:
    // data members
    shared_ptr<wxMenu> fileMenu;
    shared_ptr<wxMenu> helpMenu;
    shared_ptr<wxMenuBar> menuBar;
    
    // any class wishing to process wxWindows events must use this macro
    DECLARE_EVENT_TABLE()
};

// Define a new application type, each program should derive a class from wxApp
class MyApp : public wxApp
{
public:
    // cosntructor
    MyApp() : frame(new MyFrame("Minimal wxWindows App", wxPoint(50, 50), wxSize(450, 340))) {}
    // override base class virtuals
    // ----------------------------

    // this one is called on application startup and is a good place for the app
    // initialization (doing it here and not in the ctor allows to have an error
    // return: if OnInit() returns false, the application terminates)
    // 'Main program' equivalent: the program execution "starts" here
    virtual bool OnInit()
    {   
        // show the main application window (the frames, unlike simple controls, are not shown when
        // created initially)
        frame->Show(true);
    
        // success: wxApp::OnRun() will be called which will enter the main message
        // loop and the application will run. If we returned FALSE here, the
        // application would exit immediately.
        return true;
    }
private:
    // data members
    shared_ptr<MyFrame> frame;
};

// ----------------------------------------------------------------------------
// event tables and other macros for wxWindows
// ----------------------------------------------------------------------------

// the event tables connect the wxWindows events with the functions (event
// handlers) which process them. It can be also done at run-time, but for the
// simple menu events like this the static method is much simpler.
BEGIN_EVENT_TABLE(MyFrame, wxFrame)
    EVT_MENU(Minimal_Quit,  MyFrame::OnQuit)
    EVT_MENU(Minimal_About, MyFrame::OnAbout)
END_EVENT_TABLE()

// Create a new application object: this macro will allow wxWindows to create
// the application object during program execution (it's better than using a
// static object for many reasons) and also declares the accessor function
// wxGetApp() which will return the reference of the right type (i.e. MyApp and
// not wxApp)
IMPLEMENT_APP(MyApp)

[Martin]

Hello HighCommander4,

when I tried your program, it already crashed upon startup. I was able to fix this by moving the creation of MyFrame to the application's OnInit function. It then crashed when exiting... My guess is that it has something to do with the use of shared_ptr. Probably MyFrame does not get destroyed at the right point in time.


Hope this helps,

Martin

[ChrisBorgolte]

Your usage of shared_ptr is not consequent. You deliver the menus to the frame per shared_ptr::get() method. By default, when a frame is deleted, it deletes all its children. So the menus will be deleted by the frame and by the containing shared_ptrs.

In other words: wxWidgets handles destroying of objects by itself. The only exception I know is wxDialog.

If you want to delete a wxWindow explicitly you should use the Destroy()-method. But that is seldom neccessary.

hth
Chris

[lvdlinden]

As ChrisBorgolte mentions, wxWidgets takes care of the destruction. Check out the wiki. The manual explicitly states when wxWidgets does not take ownership of an object and the user is responsible for deleting it, for example wxClientDataContainer

[HighCommander4]

Are you telling me that the library is somehow designed such that I can create objects with new and neglect to delete them yet not get memory leaks?

This is what I am really confused about:

1) Which type of wx objects can I create on the stack?
2) Which type of wx objects can I create with new and not worry about deleting them?
3) Which type of wx objects can I create with new and have to manually delete them?

[Lucky75]

Ok,

Essentially:

- Any objects created on the stack are cleaned up automatically, as is custom with c++ (ie. no pointers or memory allocation)
- Any objects created on the heap (ie. using pointers, new, etc) need to be cleaned up by the programmer.
- Any class derived from a wxWindow is cleaned automatically when you call Destroy () or Close(). Call Destroy() on everything except frames and dialogs (or any managed window). On managed windows, use Close() instead. Then you should catch the close event and handle stuff in there. It is generally a bad idea to call delete on a window pointer. Also, do not create windows on the stack.
- All children of a wxWindow are also closed when the parent window closes. You still need to handle stuff within the child though, and ensure your destructor is working properly.
- Ensure that you delete array pointers appropriately, calling "delete[] myPointer" instead of just "delete myPointer".
- Know the difference between creating a new object and a new pointer to the object. Should you do something like the following:

Code: Select all

myNewClass::myNewClass()
{
   myClass *a = new myClass();
   myClass *b = new myClass;
   myClass *c;

   b = a;
   c = b;
}

myNewClass::~myNewClass()
{
   delete a;
   delete b;
   delete c;
}

This is very bad. For starters, you lose your pointer to "b", giving you a memory leak. Additionally, when your program exits and the destructor is called, you will get a crash since you are trying to delete the same data three times. In your assignments "b=a" and "c=b" you would be assigning both pointers to the same bit of memory on the heap, so when you call "delete b" or "delete c", it was already deleted with "delete a".


Your crash definately has something to do with your shared_ptr and how you allocate your menus/items such as "fileMenu(new wxMenu)". It is also generally not a good idea to keep your definitions and implementation together. At the very least, you should create your class definition and then implement, even if you don't want to move the definition to it's own header file.

[HighCommander4]

I understand how the stack and heap work and everything, it's just weird that wxWidgets imposes these weird restrictions like not being able to create certain objects on the stack or not being able to delete what you new-ed.

Anyways, here is my revised code. It still crashes on exit. What am I still doing wrong?

Code: Select all

// headers

#include "wx/wx.h"

// IDs for the controls and the menu commands
enum
{
    // menu items
    Minimal_Quit = 1,
    Minimal_About
};

// Define a new frame type: this is going to be our main frame
class MyFrame : public wxFrame
{
public:
    // ctor(s)
    MyFrame(const wxString& title, const wxPoint& pos, const wxSize& size,
            long style = wxDEFAULT_FRAME_STYLE) : wxFrame(NULL, -1, title, pos, size, style)
    {       
        // the "About" item should be in the help menu
        helpMenu.Append(Minimal_About, "&About...\tF1", "Show about dialog");
    
        fileMenu.Append(Minimal_Quit, "E&xit\tAlt-X", "Quit this program");
    
        // now append the freshly created menu to the menu bar...
        menuBar.Append(&fileMenu, "&File");
        menuBar.Append(&helpMenu, "&Help");
    
        // ... and attach this menu bar to the frame
        SetMenuBar(&menuBar);
    
        // create a status bar just for fun (by default with 1 pane only)
        CreateStatusBar();
        SetStatusText("Welcome to wxWindows!");
    }

    // event handlers (these functions should _not_ be virtual)
    void OnQuit(wxCommandEvent& event)
    {
        // TRUE is to force the frame to close
        Close(true);
    }
    void OnAbout(wxCommandEvent& event)
    {
        wxString msg;
        msg.Printf("This is the About dialog of the minimal sample.\nWelcome to %s", wxVERSION_STRING);
    
        wxMessageBox(msg, "About Minimal", wxOK | wxICON_INFORMATION, this);
    }

private:
    // data members
    wxMenu    fileMenu;
    wxMenu    helpMenu;
    wxMenuBar menuBar;
    
    // any class wishing to process wxWindows events must use this macro
    DECLARE_EVENT_TABLE()
};

// Define a new application type, each program should derive a class from wxApp
class MyApp : public wxApp
{
public:
    // override base class virtuals
    // ----------------------------

    // this one is called on application startup and is a good place for the app
    // initialization (doing it here and not in the ctor allows to have an error
    // return: if OnInit() returns false, the application terminates)
    // 'Main program' equivalent: the program execution "starts" here
    virtual bool OnInit()
    {   
        // create the main application window
        frame = new MyFrame("Minimal wxWindows App", wxPoint(50, 50), wxSize(450, 340));
        
        // and show it (the frames, unlike simple controls, are not shown when
        // created initially)
        frame->Show(true);
    
        // success: wxApp::OnRun() will be called which will enter the main message
        // loop and the application will run. If we returned FALSE here, the
        // application would exit immediately.
        return true;
    }
private:
    // data members
    MyFrame* frame;
};

// ----------------------------------------------------------------------------
// event tables and other macros for wxWindows
// ----------------------------------------------------------------------------

// the event tables connect the wxWindows events with the functions (event
// handlers) which process them. It can be also done at run-time, but for the
// simple menu events like this the static method is much simpler.
BEGIN_EVENT_TABLE(MyFrame, wxFrame)
    EVT_MENU(Minimal_Quit,  MyFrame::OnQuit)
    EVT_MENU(Minimal_About, MyFrame::OnAbout)
END_EVENT_TABLE()

// Create a new application object: this macro will allow wxWindows to create
// the application object during program execution (it's better than using a
// static object for many reasons) and also declares the accessor function
// wxGetApp() which will return the reference of the right type (i.e. MyApp and
// not wxApp)
IMPLEMENT_APP(MyApp)

[tan]

I understand how the stack and heap work and everything

Really?

, it's just weird that wxWidgets imposes these weird restrictions like not being able to create certain objects on the stack or not being able to delete what you new-ed.

Anyways, here is my revised code. It still crashes on exit. What am I still doing wrong?

Try this:

Code: Select all

// headers

#include "wx/wx.h"

// IDs for the controls and the menu commands
enum
{
    // menu items
    Minimal_Quit = wxID_HIGHEST,
    Minimal_About
};

// Define a new frame type: this is going to be our main frame
class MyFrame : public wxFrame
{
public:
    // ctor(s)
    MyFrame(const wxString& title, const wxPoint& pos, const wxSize& size,
            long style = wxDEFAULT_FRAME_STYLE) : wxFrame(NULL, -1, title, pos, size, style)
    {       
        menuBar = new wxMenuBar();

        // the "About" item should be in the help menu
        helpMenu = new wxMenu();
        helpMenu->Append(Minimal_About, "&About...\tF1", "Show about dialog");
    
        fileMenu = new wxMenu();
        fileMenu->Append(Minimal_Quit, "E&xit\tAlt-X", "Quit this program");
    
        // now append the freshly created menu to the menu bar...
        menuBar->Append(fileMenu, "&File");
        menuBar->Append(helpMenu, "&Help");
    
        // ... and attach this menu bar to the frame
        SetMenuBar(menuBar);
    
        // create a status bar just for fun (by default with 1 pane only)
        CreateStatusBar();
        SetStatusText("Welcome to wxWindows!");
    }

    // event handlers (these functions should _not_ be virtual)
    void OnQuit(wxCommandEvent& event)
    {
        // TRUE is to force the frame to close
        Close(true);
    }
    void OnAbout(wxCommandEvent& event)
    {
        wxString msg;
        msg.Printf("This is the About dialog of the minimal sample.\nWelcome to %s", wxVERSION_STRING);
    
        wxMessageBox(msg, "About Minimal", wxOK | wxICON_INFORMATION, this);
    }

private:
    // data members
    wxMenu*    fileMenu;
    wxMenu*    helpMenu;
    wxMenuBar* menuBar;
    // Really you don't need to save menu pointers in the member variables.

    // any class wishing to process wxWindows events must use this macro
    DECLARE_EVENT_TABLE()
};

Have a look at wx docs about menu:

Allocation strategy

All menus except the popup ones must be created on the heap. All menus attached to a menubar or to another menu will be deleted by their parent when it is deleted. As the frame menubar is deleted by the frame itself, it means that normally all menus used are deleted automatically.

[mc2r]

I understand how the stack and heap work and everything, it's just weird that wxWidgets imposes these weird restrictions like not being able to create certain objects on the stack or not being able to delete what you new-ed.

It's not really weird if you understand how the stack and heap work. The restrictiions are imposed on you by the functionality of the stack and heap afterall. They aren't some arbitrary rules made up by the developers.

-Max

[Lucky75]

you could just use managed C++ and avoid all these issues...but I hate managed c++

[HighCommander4]

I do understand how the stack and the heap work. I have been programming in C++ for years. One of first and most important things I learned about C++ is that whatever you create with new, you must destroy with delete. These libraries deviate from that, hence weird.

Anyways, in my last version of the code, the MyFrame object is created on the heap, since I am new-ing it. Does that not mean that its member variables, the menu and the menu bar, are also created on the heap? Or do they mean that I must explicitly create all menus and menubars with new? If so, it contradicts what they say on the wiki:

Classes that are not derived from wxWindow or wxSizer
Classes that are not derived off wxWindow or wxSizer can be created and deleted like normal C++ classes, as described above, either on the stack or on the heap.

wxMenu is not derived from wxWindow or wxSizer! (wxMenuBar, granted, is - that was my mistake).

[Martin]

Some functions (or the objects they belong to) take possession of the objects passed to them via pointer parameters. They are then responsible for the deletion of those objects.
The problem is: you have to know how the function you are calling behaves (which can usually only be figured out by reading the, hopefully useful, function description, or by looking at the source code).

[tan]

Or do they mean that I must explicitly create all menus and menubars with new? If so, it contradicts what they say on the wiki:

Classes that are not derived from wxWindow or wxSizer
Classes that are not derived off wxWindow or wxSizer can be created and deleted like normal C++ classes, as described above, either on the stack or on the heap.

wxMenu is not derived from wxWindow or wxSizer! (wxMenuBar, granted, is - that was my mistake).

Read wx documentation http://www.wxwindows.org/manuals/stable ... tml#wxmenu:

All menus attached to a menubar or to another menu will be deleted by their parent when it is deleted.

IMO it is perfectly clear.

[HighCommander4]

So basically you have to look at every class individually to see whether or not it can be created on the stack and every function to see whether or not it takes possession of an object passed as parameter so you can figure out how you are allowed to create or delete the objects without any memory leaks or things being deleted twice?

This smells like a memory management disaster...

Why couldn't they just design ordinary objects like std::string and std::vector that you can create on the the stack OR on the heap whichever your like, where you delete what you new-ed and any special initialization and deinitialization happens in the constructor and the destructor, instead of overloading operators new and delete and placing restrictions on how objects can be created?

[Auria]

So basically you have to look at every class individually to see whether or not it can be created on the stack and every function to see whether or not it takes possession of an object passed as parameter so you can figure out how you are allowed to create or delete the objects without any memory leaks or things being deleted twice?

I wonder how you manage to write code without docs...

and actually you don't need to check the docs everytime, wxWidgets always acts the same way, parents delete their children