Author Topic: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly  (Read 13431 times)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
I found a very annoying issue. Here are steps to reproduce:
1, open C::B
2, create a simple wxWidgets app with the wizard(choose wxFrame based, wxSmith as the GUI designer)
3, add a wxAuiManager to the main Frame
4, save all
5, close C::B

The result is: I see the codeblocks.exe process is still in the Windows task manager(its memory size is about 35M to 40M), I have to kill this process.

Note, if you don't have wxAuiManager object in the wxFrame, you don't have such issue.

I have tested this steps under:
1, 32bit official C::B 17.12 under Win7(64bit), which is wx2.8.12 based.
2, 32bit C::B trunk build myself with wx3.12, gcc 7.2.
Both have such issue.

I try to debug this issue, but it looks like the process hang issue after the function call: void MainFrame::OnApplicationClose(wxCloseEvent& event), so it is hard to debug. Also, since C::B is halt (it stalls inside the Win32 wait object functions), I don't have a call stack here.

This issue firstly happens in a complex wxsmith project, but I just reduce the wxsmith and finally see that a single wxAuiManager object in wxsmith will cause this issue.

This issue is not related to code generation, because I event don't need to build the cbp project, just open the wxsmith gui designer(the preview window) will cause this issue.

Any ideas?
 
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #1 on: February 11, 2019, 06:40:35 pm »
1. Does reverting rev11530 changes anything?
2. Have you tried to reproduce the problem in some wx sample?
3. Have you tried with a debug wx build?
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #2 on: February 11, 2019, 10:38:30 pm »
Could it be this:
https://sourceforge.net/p/codeblocks/tickets/789/
Quote
2) AuiManager::UnInit() has to be added to the destructor of the window code
in the destructor of the preview?

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #3 on: February 12, 2019, 12:21:02 am »
Could it be this:
https://sourceforge.net/p/codeblocks/tickets/789/
Quote
2) AuiManager::UnInit() has to be added to the destructor of the window code
in the destructor of the preview?

There is a UnInit() function call in the preview wxAuiManager derived object: in file: src\plugins\contrib\wxSmithAui\wxAuiManager\wxSmithAuiManager.h
Code
#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?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #4 on: February 12, 2019, 12:28:15 am »
1. Does reverting rev11530 changes anything?
I'm not sure, but mostly I think it is not related to this commit.
Quote
SHA-1: 28734b8204bff66d8a9d6b8e7bb2f4eb8ad83561

* * 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
But the issue happens in official C::B 17.12 version(r11256), which is long before commit r11530.


Quote
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.

Quote
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.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #5 on: February 12, 2019, 12:52:57 am »
...
There is a UnInit() function call in the preview wxAuiManager derived object: in file: src\plugins\contrib\wxSmithAui\wxAuiManager\wxSmithAuiManager.h
Code
#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?

Oh, bad/good news, I just set a breakpoint in the  UnInit(); line, and When I close debugee C::B, this line does not reached. and debugee C::B now go to endless wait status. So my guess is that why this function call is missing here?

EDIT:
The UnInit() is called after I add one page in the wxAuiManager. When there is no items controlled by wxAuiManager, it is not called.(in the wxsmith preview, wxAuiManager object only constructed when there is at least one item inside it)
« Last Edit: February 12, 2019, 12:59:52 am by ollydbg »
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #6 on: February 12, 2019, 03:31:24 pm »
Good news, I think I have find the reason of this issue.

With this patch, I can avoid the hang process when C::B exits. (especially follow my 5 steps in my first post in this thread)

under function: bool wxsAuiManager::OnCanAddToParent(wxsParent* Parent,bool ShowMessage), it need to test whether a wxAuiManager can be child of a Parent window.
Code
 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);
 }
 


I'm really don't know what does this code means?
Code
new wxFrame(0,-1,wxEmptyString)
This means we allocate a wxFrame under the desktop? The first argument is 0 means the desktop?

I see there are three places which use "new wxFrame(0,...)" in our C::B's source code, and they all exists under the folder:
two places in src\plugins\contrib\wxSmithAui\wxAuiManager\wxsAuiManager.cpp
one place in src\plugins\contrib\wxSmithAui\wxAuiToolBar\wxsAuiToolBar.cpp

I guess this usage of create a wxFrame is wrong, do you have any ideas how to fix this issue? Thanks.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline sodev

  • Regular
  • ***
  • Posts: 497
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #7 on: February 12, 2019, 05:45:33 pm »
The first argument is actually a pointer and passing a nullptr means to create frame without parent which is fully independend.

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.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #8 on: February 13, 2019, 01:40:10 am »
The first argument is actually a pointer and passing a nullptr means to create frame without parent which is fully independend.

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.
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)
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #9 on: February 13, 2019, 02:35:11 am »
I change the code like below:

Code
    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();

It looks like this code does not make C::B endless wait on exit. :)

I will change the other "dangling" frame places in wxAui related code in wxsmith.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #10 on: February 13, 2019, 02:47:54 am »
Code
 
 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 )
     {


This is the full patch. OK to commit to trunk?
« Last Edit: February 13, 2019, 02:49:26 am by ollydbg »
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline sodev

  • Regular
  • ***
  • Posts: 497
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #11 on: February 13, 2019, 05:12:32 am »
Dont use 0 but nullptr, dont use -1 but wxID_ANY, dont use raw pointer but something managed, something like:
Code
auto 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.

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?

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #12 on: February 13, 2019, 06:59:13 am »
Dont use 0 but nullptr, dont use -1 but wxID_ANY, dont use raw pointer but something managed, something like:
Code
auto 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.
Thanks for the suggestion, and in-fact I'm not good at writing lambda expressions  :), here is improved patch as your suggested.
Code
 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 )
     {



Quote
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. :(
« Last Edit: February 13, 2019, 07:01:58 am by ollydbg »
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #13 on: February 13, 2019, 09:22:56 am »
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.
Can you please not use auto? In this case it doesn't make sense to use it.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #14 on: February 13, 2019, 10:56:09 am »
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.

Quote
Can you please not use auto? In this case it doesn't make sense to use it.

Here is the updated patch, any comments?
Code
 .../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 )
     {


EDIT:
I don't think add the same part in two cpp file is a good idea.
Code
+struct TempFrameDeleter
+{
+    void operator()(wxFrame* frame)
+    {
+        frame->Destroy();
+    }
+};
+
+using TempFrameUniquePtr = std::unique_ptr<wxFrame, TempFrameDeleter>;

EDIT2:
Follow this page: c++ - Simple custom deleter lambda supplied to std::unique_ptr: why use by-reference capture default ([&]) over no-capture ([])? - Stack Overflow
This is more compact way of define the unique_ptr, OK with below code?
Code
        std::unique_ptr<wxFrame, std::function<void(wxFrame*)>> tempFrame(new wxFrame(nullptr, wxID_ANY, wxEmptyString),
                                                                          [&](wxFrame* frame){ frame->Destroy(); });  // deleter
« Last Edit: February 13, 2019, 03:00:29 pm by ollydbg »
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #15 on: February 13, 2019, 08:02:04 pm »
1. Do not use lambda captures if you don't understand them.
2. Why do you need a capture in this case? The frame is passed as a parameter.
3. What does Temp mean? To me it is the same as using My, Mine, Random, Some, etc.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #16 on: February 14, 2019, 02:02:27 am »
1. Do not use lambda captures if you don't understand them.
Oh, I need to learn it. Should be easy.  :)
Quote
2. Why do you need a capture in this case? The frame is passed as a parameter.
You are correct, no need capture here.

Quote
3. What does Temp mean? To me it is the same as using My, Mine, Random, Some, etc.
What about shortLiveFrame or transientFrame?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #17 on: February 14, 2019, 07:06:57 pm »
transientFrame has different meaning in some OS/wms.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #18 on: February 16, 2019, 06:23:45 am »
transientFrame has different meaning in some OS/wms.
OK, thanks.

I now use the "shortLiveFrame". I have committed the patch to trunk now(r11571). Thanks all guys for your help.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #19 on: February 17, 2019, 02:15:33 pm »
Hi,
i can not build the latest C::B trunk on linux, because of the error message
Code
function is not a member of std
To fix this i have to add
Code
#include <functor>
at the top of the edited files

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #20 on: February 17, 2019, 03:10:26 pm »
Hi,
i can not build the latest C::B trunk on linux, because of the error message
Code
function is not a member of std
To fix this i have to add
Code
#include <functor>
at the top of the edited files
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.
« Last Edit: February 17, 2019, 03:18:46 pm by ollydbg »
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #21 on: February 17, 2019, 03:55:18 pm »
@bluehazzard: There is no <functor> header it is <functional> ...
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #22 on: February 17, 2019, 04:05:41 pm »
@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?
I will add the headers only in the cpp source files where it is needed

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #23 on: February 17, 2019, 04:14:46 pm »
Well here we are.... I have commited with the wrong message text... Is there a way to edit the commit message?

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #24 on: February 17, 2019, 04:21:43 pm »
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.

PS: I remember I did this several years ago. Now, I don't use TortoiseSVN for a long time. :)
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #25 on: February 17, 2019, 04:27:37 pm »
Ok, it worked... Thank you!

Offline sodev

  • Regular
  • ***
  • Posts: 497
Re: wxAuiManager preview issue under wxSmith cause C::B can't exit correctly
« Reply #26 on: February 18, 2019, 12:38:13 am »
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 :).