Issue with destroying user-closed window (MS Windows)

[4ggre5510n]

Greetings!

I've came upon some issues with closing and destroying windows already closed by user.

Here is sample code of invoking wxFrame d-tor after user closed the window:

Code: Select all

#include <wx/wx.h>

class app : public wxApp {
	wxFrame* mainFrame;
public:
	bool OnInit() {
		mainFrame = new wxFrame(NULL, wxID_ANY, L"Sample");
		mainFrame->Show();
		return true;
	}

	app() : mainFrame(NULL), wxApp() { }

	~app() {
		// Regular, default situation: class is destroying object that was created by it .
		delete mainFrame; /** Applications __CRASHES__ here [MinGW] **/

		// Also, some of other wxWindow or wxFrame closing/destryoing functions wouldn't work ( i mean would crash app )
		// mainFrame->Destroy(); mainFrame->Close();

		// but also some simple examinations, like
		// mainFrame->IsBeingDeleted();
	}

};
IMPLEMENT_APP(app);

For me it's pretty obvious that wxApp _should_ destroy object created by her...
Isn't there some enormous mistake inside wxFrame/wxWindow class? Is the destructor already called?


Regular, code-invoked deletion, like:

Code: Select all

frame->Destroy(); delete frame;

Works just fine.

I've came up with an idea to check withing a function is a frame/window already deleted. It's (undocumented...) wxWindow->GetHWND(); It does returns 0 if the window is destroyed.
Would work fine with functions like Destroy()....
But I really wanna clear up my pointers before destructing my class [ of course above code is just simple example, I have pretty much more complex class hierarchy going in my project ]. Or are they already cleared, from inside-wxWindow-class?


I'm using newest MinGW under Windows 7.
wxWidgets: 2.93. Monolithic, dynamic-library build.


Thank you in advance for your answers!

[doublemax]

wxFrames destroy themselves. Never delete a wxFrame *

http://docs.wxwidgets.org/stable/wx_win ... rview.html

[4ggre5510n]

Ok, thanks, that's clear up things a lot ;]

But, isn't that wrong OOP philosophy? Shouldn't the class creating object be responsible for deleting it?

Anyway:
Is that any better way to check if window is already destroyed then (undocumented) wxWindow->GetHWND() ?

[PB]

Is that any better way to check if window is already destroyed then (undocumented) wxWindow->GetHWND() ?

I don't know about checking for it being destroyed (what happens if it has been already deleted and the pointer is not valid?) but if you want to be notified about a window's destruction, you can Bind its WindowDestroyEvent, that's usually good enough.

Code: Select all

#include <wx/wx.h>

class MyChildFrame : public wxFrame
{
public:
    MyChildFrame(wxFrame* parent, const wxString& title)
        : wxFrame(parent, wxID_ANY, title, wxDefaultPosition, wxSize(300,100))
    {
        SetName(title);
    }
};

class MyFrame : public wxFrame
{
public:
    MyFrame()
        : wxFrame(NULL, wxID_ANY, _("Test"))
    {
        wxMenu *frameMenu = new wxMenu;
        frameMenu->Append(wxID_NEW, _("&New frame\tCtrl+N"));
        wxMenuBar *menuBar = new wxMenuBar();
        menuBar->Append(frameMenu, _("&Frame"));
        SetMenuBar(menuBar);
        
        wxBoxSizer* bSizer = new wxBoxSizer(wxVERTICAL);

        m_logCtrl = new wxTextCtrl(this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, wxTE_MULTILINE);
        bSizer->Add( m_logCtrl, 1, wxALL|wxEXPAND, 0 );

        SetSizer(bSizer);
        Layout();
        Centre();

        Bind(wxEVT_COMMAND_MENU_SELECTED, &MyFrame::OnNewFrame, this, wxID_NEW);        
    }	
private:
    wxTextCtrl* m_logCtrl;
    
    void OnNewFrame(wxCommandEvent& WXUNUSED(event))
    {
        static int framesCreated = 0;       
        wxString title(wxString::Format("Child frame #%d", ++framesCreated));

        MyChildFrame* cf = new MyChildFrame(this, title);        
        cf->Bind(wxEVT_DESTROY, &MyFrame::OnChildFrameDestroyed, this);
        cf->Show();        
    }
    void OnChildFrameDestroyed(wxWindowDestroyEvent& event)
    {        
        // you can access only wxWindow's methods here, as the derived destructors may have been already called
        *m_logCtrl << event.GetWindow()->GetName() << " is being destroyed\n";
    }
};

/**** MyApp ****/
class MyApp : public wxApp
{
public:	
	virtual bool OnInit()
	{
		if (!wxApp::OnInit())
			return false;       	

        MyFrame* frame = new MyFrame();
        frame ->Show();

        return true;
	}
};

IMPLEMENT_APP(MyApp)

[doublemax]

But, isn't that wrong OOP philosophy? Shouldn't the class creating object be responsible for deleting it?

Technically yes. But would you want to manually delete any control, any wxButton, any wxSizer? Probably not.

Is that any better way to check if window is already destroyed then (undocumented) wxWindow->GetHWND() ?

If the window is already destroyed, in best case this call would crash, in worst case it would return the random content of the memory that used to belong to a wxWindow.

For what would you need that anyway?

[4ggre5510n]

Is that any better way to check if window is already destroyed then (undocumented) wxWindow->GetHWND() ?

If the window is already destroyed, in best case this call would crash, in worst case it would return the random content of the memory that used to belong to a wxWindow.

That's good point
If I understand correctly: Close() invoked by user, processed without "veto" ( default way ), ends up calling Destroy() and freeing memory ( delete this; ), right ?

For what would you need that anyway?

I create small class that creates splash window (application logo + status bar) when application is initialized.
Class loads up bunch of data, destroys splash window then calls wxMyApp::_Initialization_Done, which creates Main Frame.

However, user can dismiss splash window ( nothing more annoying then always-on-top window that you cannot hide... ).

In the destructor (or rather the function catching events from worker threads) of the class creating splash window, I need to know is this window already destroyed by user. If it is - do nothing. If it's still there - destroy it. Then create Main Frame.

I guess the only way is to register destroy event, right (since user Closing() frame actually destroys it and frees all memory associated, right? ) ?

[doublemax]

However, user can dismiss splash window ( nothing more annoying then always-on-top window that you cannot hide... ).

What i would do is that the user action just hides the window, then you safely destroy it in any case.

[4ggre5510n]

What i would do is that the user action just hides the window, then you safely destroy it in any case.

Well, this is just not pretty work-around...
What if this splash window would present memory-intensive 3d animation ( poor example, but... )? I know it won't be ever showed again, why not free memory right away?

Registering wxDestroyEvent is - I guess - better.

But I was watching at the wxIDManager and was wondering - does ID reserved for given window is freed from wxWindow destructor? If no - is there any way to access currently reserved IDs?

If it won't work, I will go with Bind'ing wxDestroyEvent.
But I believe devs team should rethink automate deletion of wxTopLevelWindow.
As stated before - it's good thing have this for all child widgets. But there should be given option for more control of TopLevelWindow. Explicit deleting wouldn't be much problem. Maybe someone would want to look into this... Or I'm just a grim, young noob ;]
I know Qt enables options to deleting whole c++ object or just underlying native OS window.

[doublemax]

Registering wxDestroyEvent is - I guess - better.

Sounds overly complicated for me, but it's your choice.

Another option: There a global list "wxTopLevelWindows" that contains all toplevelwindows. So you could try something like this:

Code: Select all

if( wxTopLevelWindows.IndexOf( yourwindowhere ) != wxNOT_FOUND )
{
  // window exists, do something here
}

Untested though.