2) AuiManager::UnInit() has to be added to the destructor of the window codein the destructor of the preview?
Could it be this:
https://sourceforge.net/p/codeblocks/tickets/789/Quote2) AuiManager::UnInit() has to be added to the destructor of the window codein the destructor of the preview?
#ifndef WXSMITHAUIMANAGER_H
#define WXSMITHAUIMANAGER_H
#include <wx/aui/aui.h>
#include <wx/wx.h>
#include <wx/event.h>
#include <prep.h>
class wxSmithAuiManager : public wxAuiManager
{
public:
wxSmithAuiManager(wxWindow* managed_wnd = NULL, unsigned int flags = wxAUI_MGR_DEFAULT)
: wxAuiManager(managed_wnd, flags)
{
Connect(wxEVT_DESTROY,(wxObjectEventFunction)&wxSmithAuiManager::OnDestroy);
}
virtual ~wxSmithAuiManager() {}
void OnDestroy(cb_unused wxWindowDestroyEvent& event)
{
UnInit();
}
protected:
private:
};
#endif // WXSMITHAUIMANAGER_H
1. Does reverting rev11530 changes anything?I'm not sure, but mostly I think it is not related to this commit.
SHA-1: 28734b8204bff66d8a9d6b8e7bb2f4eb8ad83561But the issue happens in official C::B 17.12 version(r11256), which is long before commit r11530.
* * batch build: Fix segmentation fault after batch build (ticket #738)
> The problem happens because there is a Manager::Yield() in
MainFrame::OnApplicationClose. This call to yield makes wxWidgets execute
all events in its event queue and unfortunately there are events related
to delayed destruction of the current window which get executed at a bad
time.
> If I remove the Yield call the bug goes away. Not sure what would break
by this change, but we'll probably see.
> I've tested if the APP_SHUTDOWN event is still sent and received. I found
no problems with this.
git-svn-id: https://svn.code.sf.net/p/codeblocks/code/trunk@11530 2a5c6006-c6dd-42ca-98ab-0921f2732cef
2. Have you tried to reproduce the problem in some wx sample?The issue happens in preview of the wxSmith, the generated executable by C::B doesn't have such issue.
3. Have you tried with a debug wx build?Note tried yet, building wx3.12 under gcc 7.2 is quite slow, will take some time to try it.
...
There is a UnInit() function call in the preview wxAuiManager derived object: in file: src\plugins\contrib\wxSmithAui\wxAuiManager\wxSmithAuiManager.hCode#ifndef WXSMITHAUIMANAGER_H
#define WXSMITHAUIMANAGER_H
#include <wx/aui/aui.h>
#include <wx/wx.h>
#include <wx/event.h>
#include <prep.h>
class wxSmithAuiManager : public wxAuiManager
{
public:
wxSmithAuiManager(wxWindow* managed_wnd = NULL, unsigned int flags = wxAUI_MGR_DEFAULT)
: wxAuiManager(managed_wnd, flags)
{
Connect(wxEVT_DESTROY,(wxObjectEventFunction)&wxSmithAuiManager::OnDestroy);
}
virtual ~wxSmithAuiManager() {}
void OnDestroy(cb_unused wxWindowDestroyEvent& event)
{
UnInit();
}
protected:
private:
};
#endif // WXSMITHAUIMANAGER_H
So, it will be called when the preview window closed?
src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
index bbd3d967..60735822 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
@@ -640,14 +640,14 @@ bool wxsAuiManager::OnCanAddToParent(wxsParent* Parent,bool ShowMessage)
wxMessageBox(_("wxAuiManager can't be added to a sizer. Add panels first."));
return false;
}
-
+#if 0
if ( !wxDynamicCast(Parent->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxWindow) )
{
if ( ShowMessage )
wxMessageBox(_("wxAuiManager can only be added to a wxWindow descendant."));
return false;
}
-
+#endif // 0
return wxsParent::OnCanAddToParent(Parent,ShowMessage);
}
new wxFrame(0,-1,wxEmptyString)
The first argument is actually a pointer and passing a nullptr means to create frame without parent which is fully independend.Thanks for the explanation. Now, I will try to call the Destroy() function of the frame objects when this function exits. (Documents are here: https://docs.wxwidgets.org/3.1/overview_windowdeletion.html)
I think the reason for CodeBlocks to not close is that no one ever destroys these temporary frames (unless that BuildPreview method does it internally) and the "dangling" frames keep wxWidgets alive.
wxFrame * tempFrame = new wxFrame(0,-1,wxEmptyString);
if ( !wxDynamicCast(Parent->BuildPreview(tempFrame,0),wxWindow) )
{
if ( ShowMessage )
wxMessageBox(_("wxAuiManager can only be added to a wxWindow descendant."));
tempFrame->Destroy();
return false;
}
tempFrame->Destroy();
src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp | 11 +++++++----
src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp | 4 +++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
index bbd3d967..3b9bfcbb 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
@@ -640,14 +640,15 @@ bool wxsAuiManager::OnCanAddToParent(wxsParent* Parent,bool ShowMessage)
wxMessageBox(_("wxAuiManager can't be added to a sizer. Add panels first."));
return false;
}
-
- if ( !wxDynamicCast(Parent->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxWindow) )
+ wxFrame * tempFrame = new wxFrame(0,-1,wxEmptyString);
+ if ( !wxDynamicCast(Parent->BuildPreview(tempFrame,0),wxWindow) )
{
if ( ShowMessage )
wxMessageBox(_("wxAuiManager can only be added to a wxWindow descendant."));
+ tempFrame->Destroy();
return false;
}
-
+ tempFrame->Destroy();
return wxsParent::OnCanAddToParent(Parent,ShowMessage);
}
@@ -665,7 +666,8 @@ void wxsAuiManager::OnAddChildQPP(wxsItem* Child,wxsAdvQPP* QPP)
if ( ChildExtra->m_FirstAdd )
{
ChildExtra->m_FirstAdd = false;
- if ( wxDynamicCast(Child->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxAuiToolBar) )
+ wxFrame * tempFrame = new wxFrame(0,-1,wxEmptyString);
+ if ( wxDynamicCast(Child->BuildPreview(tempFrame,0),wxAuiToolBar) )
{
ChildExtra->m_StandardPane = wxsAuiPaneInfoExtra::ToolbarPane;
ChildExtra->m_DockableFlags = wxsAuiDockableProperty::Dockable;
@@ -680,6 +682,7 @@ void wxsAuiManager::OnAddChildQPP(wxsItem* Child,wxsAdvQPP* QPP)
if ( ChildExtra->m_Layer == 0 ) ChildExtra->m_Layer = 10;
NotifyPropertyChange();
}
+ tempFrame->Destroy();
}
}
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
index 66b67edc..7f0f8eff 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
@@ -126,7 +126,9 @@ wxsAuiToolBar::~wxsAuiToolBar()
bool wxsAuiToolBar::OnCanAddChild(wxsItem* Item,bool ShowMessage)
{
- bool IsControl = wxDynamicCast(Item->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxControl);
+ wxFrame * tempFrame = new wxFrame(0,-1,wxEmptyString);
+ bool IsControl = wxDynamicCast(Item->BuildPreview(tempFrame,0),wxControl);
+ tempFrame->Destroy();
bool IsAuiToolBarItem = Item->GetClassName().Contains(_T("wxAuiToolBar"));
if ( !IsControl && !IsAuiToolBarItem )
{
auto tempFrame = std::shared_ptr<wxFrame>(new wxFrame(nullptr, wxID_ANY, wxEmptyString), [](wxFrame* frame) { frame->Destroy(); });
Dont use 0 but nullptr, dont use -1 but wxID_ANY, dont use raw pointer but something managed, something like:Thanks for the suggestion, and in-fact I'm not good at writing lambda expressions :), here is improved patch as your suggested.Codeauto tempFrame = std::shared_ptr<wxFrame>(new wxFrame(nullptr, wxID_ANY, wxEmptyString), [](wxFrame* frame) { frame->Destroy(); });
This way you dont need explicit Destroy() calls. Would be better to use std::unique_ptr, but there the deleter is part of the signature and i currently dont know how to use that with a nice lambda expression and dont have a compiler on my cell phone to play around with :D.
src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp | 9 ++++++---
src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp | 4 +++-
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
index bbd3d967..3cf86b3e 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
@@ -640,8 +640,9 @@ bool wxsAuiManager::OnCanAddToParent(wxsParent* Parent,bool ShowMessage)
wxMessageBox(_("wxAuiManager can't be added to a sizer. Add panels first."));
return false;
}
-
- if ( !wxDynamicCast(Parent->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxWindow) )
+ auto tempFrame = std::shared_ptr<wxFrame>(new wxFrame(nullptr, wxID_ANY, wxEmptyString),
+ [](wxFrame* frame) {frame->Destroy();}); //deleter
+ if ( !wxDynamicCast(Parent->BuildPreview(tempFrame.get(),0),wxWindow) )
{
if ( ShowMessage )
wxMessageBox(_("wxAuiManager can only be added to a wxWindow descendant."));
@@ -665,7 +666,9 @@ void wxsAuiManager::OnAddChildQPP(wxsItem* Child,wxsAdvQPP* QPP)
if ( ChildExtra->m_FirstAdd )
{
ChildExtra->m_FirstAdd = false;
- if ( wxDynamicCast(Child->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxAuiToolBar) )
+ auto tempFrame = std::shared_ptr<wxFrame>(new wxFrame(nullptr, wxID_ANY, wxEmptyString),
+ [](wxFrame* frame) {frame->Destroy();}); //deleter
+ if ( wxDynamicCast(Child->BuildPreview(tempFrame.get(),0),wxAuiToolBar) )
{
ChildExtra->m_StandardPane = wxsAuiPaneInfoExtra::ToolbarPane;
ChildExtra->m_DockableFlags = wxsAuiDockableProperty::Dockable;
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
index 66b67edc..af2adb70 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
@@ -126,7 +126,9 @@ wxsAuiToolBar::~wxsAuiToolBar()
bool wxsAuiToolBar::OnCanAddChild(wxsItem* Item,bool ShowMessage)
{
- bool IsControl = wxDynamicCast(Item->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxControl);
+ auto tempFrame = std::shared_ptr<wxFrame>(new wxFrame(nullptr, wxID_ANY, wxEmptyString),
+ [](wxFrame* frame) {frame->Destroy();}); //deleter
+ bool IsControl = wxDynamicCast(Item->BuildPreview(tempFrame.get(),0),wxControl);
bool IsAuiToolBarItem = Item->GetClassName().Contains(_T("wxAuiToolBar"));
if ( !IsControl && !IsAuiToolBarItem )
{
But beside that, why does the code even create these temporary frames and is it really safe to destroy them right after the method call? Why to create them at all then?I don't know such logic here either. :(
Can you add comments what is the purpose of the shared pointers? Why aren't you using a unique_ptr? I don't see any sharing happening here.Oh, thanks for the review, it is a copy-paste error, it definitely should be std::unique_ptr.
Can you please not use auto? In this case it doesn't make sense to use it.
.../contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp | 19 ++++++++++++++++---
.../contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp | 13 ++++++++++++-
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
index bbd3d967..5a66636c 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiManager/wxsAuiManager.cpp
@@ -27,6 +27,16 @@
using namespace wxsFlags;
+struct TempFrameDeleter
+{
+ void operator()(wxFrame* frame)
+ {
+ frame->Destroy();
+ }
+};
+
+using TempFrameUniquePtr = std::unique_ptr<wxFrame, TempFrameDeleter>;
+
namespace
{
#include "../images/wxsAuiManager16.xpm"
@@ -640,8 +650,8 @@ bool wxsAuiManager::OnCanAddToParent(wxsParent* Parent,bool ShowMessage)
wxMessageBox(_("wxAuiManager can't be added to a sizer. Add panels first."));
return false;
}
-
- if ( !wxDynamicCast(Parent->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxWindow) )
+ TempFrameUniquePtr tempFrame(new wxFrame(nullptr, wxID_ANY, wxEmptyString));
+ if ( !wxDynamicCast(Parent->BuildPreview(tempFrame.get(),0),wxWindow) )
{
if ( ShowMessage )
wxMessageBox(_("wxAuiManager can only be added to a wxWindow descendant."));
@@ -651,6 +661,8 @@ bool wxsAuiManager::OnCanAddToParent(wxsParent* Parent,bool ShowMessage)
return wxsParent::OnCanAddToParent(Parent,ShowMessage);
}
+
+
void wxsAuiManager::OnAddChildQPP(wxsItem* Child,wxsAdvQPP* QPP)
{
wxsParent::OnAddChildQPP(Child,QPP);
@@ -665,7 +677,8 @@ void wxsAuiManager::OnAddChildQPP(wxsItem* Child,wxsAdvQPP* QPP)
if ( ChildExtra->m_FirstAdd )
{
ChildExtra->m_FirstAdd = false;
- if ( wxDynamicCast(Child->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxAuiToolBar) )
+ TempFrameUniquePtr tempFrame(new wxFrame(nullptr, wxID_ANY, wxEmptyString));
+ if ( wxDynamicCast(Child->BuildPreview(tempFrame.get(),0),wxAuiToolBar) )
{
ChildExtra->m_StandardPane = wxsAuiPaneInfoExtra::ToolbarPane;
ChildExtra->m_DockableFlags = wxsAuiDockableProperty::Dockable;
diff --git a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
index 66b67edc..3870d1cb 100644
--- a/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
+++ b/src/plugins/contrib/wxSmithAui/wxAuiToolBar/wxsAuiToolBar.cpp
@@ -32,6 +32,16 @@
using namespace wxsFlags;
+struct TempFrameDeleter
+{
+ void operator()(wxFrame* frame)
+ {
+ frame->Destroy();
+ }
+};
+
+using TempFrameUniquePtr = std::unique_ptr<wxFrame, TempFrameDeleter>;
+
//(*Headers(wxsAuiToolBarParentQP)
#include <wx/checkbox.h>
#include <wx/sizer.h>
@@ -126,7 +136,8 @@ wxsAuiToolBar::~wxsAuiToolBar()
bool wxsAuiToolBar::OnCanAddChild(wxsItem* Item,bool ShowMessage)
{
- bool IsControl = wxDynamicCast(Item->BuildPreview(new wxFrame(0,-1,wxEmptyString),0),wxControl);
+ TempFrameUniquePtr tempFrame(new wxFrame(nullptr, wxID_ANY, wxEmptyString));
+ bool IsControl = wxDynamicCast(Item->BuildPreview(tempFrame.get(),0),wxControl);
bool IsAuiToolBarItem = Item->GetClassName().Contains(_T("wxAuiToolBar"));
if ( !IsControl && !IsAuiToolBarItem )
{
+struct TempFrameDeleter
+{
+ void operator()(wxFrame* frame)
+ {
+ frame->Destroy();
+ }
+};
+
+using TempFrameUniquePtr = std::unique_ptr<wxFrame, TempFrameDeleter>;
std::unique_ptr<wxFrame, std::function<void(wxFrame*)>> tempFrame(new wxFrame(nullptr, wxID_ANY, wxEmptyString),
[&](wxFrame* frame){ frame->Destroy(); }); // deleter
1. Do not use lambda captures if you don't understand them.Oh, I need to learn it. Should be easy. :)
2. Why do you need a capture in this case? The frame is passed as a parameter.You are correct, no need capture here.
3. What does Temp mean? To me it is the same as using My, Mine, Random, Some, etc.What about shortLiveFrame or transientFrame?
transientFrame has different meaning in some OS/wms.OK, thanks.
function is not a member of std
#include <functor>
Hi,Sorry about that, because I don't use Linux most of the time. This does not happen on my Windows GCC 7.2. I think you can push your changes. Thanks.
i can not build the latest C::B trunk on linux, because of the error messageCodeTo fix this i have to addfunction is not a member of std
Codeat the top of the edited files#include <functor>
@bluehazzard: There is no <functor> header it is <functional> ...yes, my fault. It is <functional>
Sorry about that, because I don't use Linux most of the time. This does not happen on my Windows GCC 7.2. I think you can push your changes. Thanks.I think this is because windows uses precompiled headers? And linux not?
Well here we are.... I have commited with the wrong message text... Is there a way to edit the commit message?Yes, you can edit the commit message. Under Windows, you can do this in TortoiseSVN, and right click of that commit, it will shown in context menu.
I think this is because windows uses precompiled headers? And linux not?More likely that a different code path pulls in different transitive includes, thats why you should always make your includes self contained :).