Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#15698 closed defect (fixed)

Removing static box sizer deletes all its contents

Reported by: Hanmac Owned by:
Priority: high Milestone: 3.0.1
Component: GUI-all Version: 3.0.0
Keywords: regression Cc:
Blocked By: Blocking:
Patch: yes

Description

when in the widgets sample, in the section "Static"
when there is some setting is changed it crashs with an Segfault, there is the bt:

0 0x00007ffff6762a48 in main_arena () from /lib/x86_64-linux-gnu/libc.so.6
1 0x0000000000479177 in StaticWidgetsPage::CreateStatic() ()
2 0x00007ffff6cf69be in wxAppConsoleBase::CallEventHandler(wxEvtHandler*, wxEventFunctor&, wxEvent&) const () from wxWidgets/lib/libwx_baseu-3.1.so.0
3 0x00007ffff6eeeeb2 in wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) () from wxWidgets/lib/libwx_baseu-3.1.so.0
4 0x00007ffff6ef0f43 in wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) () from wxWidgets/lib/libwx_baseu-3.1.so.0
5 0x00007ffff6ef0fad in wxEvtHandler::TryHereOnly(wxEvent&) () from wxWidgets/lib/libwx_baseu-3.1.so.0
6 0x00007ffff6ef1023 in wxEvtHandler::ProcessEventLocally(wxEvent&) () from wxWidgets/lib/libwx_baseu-3.1.so.0
7 0x00007ffff6ef1085 in wxEvtHandler::ProcessEvent(wxEvent&) () from wxWidgets/lib/libwx_baseu-3.1.so.0
8 0x00007ffff75e7468 in wxWindowBase::TryAfter(wxEvent&) () from wxWidgets/lib/libwx_gtk3u_core-3.1.so.0
9 0x00007ffff6eef037 in wxEvtHandler::SafelyProcessEvent(wxEvent&) () from wxWidgets/lib/libwx_baseu-3.1.so.0
10 0x00007ffff74906a2 in gtk_checkbox_toggled_callback () from wxWidgets/lib/libwx_gtk3u_core-3.1.so.0

i think the problem lies in "StaticWidgetsPage::CreateStatic" but i couldnt found it yet

Change History (5)

comment:1 Changed 7 years ago by pcor

  • Status changed from new to confirmed

The following change is enough to avoid the crash, but I have not looked carefully enough to know if it is the correct fix.

Index: samples/widgets/static.cpp
===================================================================
--- samples/widgets/static.cpp	(revision 75284)
+++ samples/widgets/static.cpp	(working copy)
@@ -382,8 +382,6 @@
 
     if ( m_sizerStatBox )
     {
-        // delete m_sizerStatBox; -- deleted by Remove()
-        m_sizerStatic->Remove(m_sizerStatBox);
         delete m_statText;
 #if wxUSE_MARKUP
         delete m_statMarkup;
@@ -391,6 +389,8 @@
 #if wxUSE_STATLINE
         delete m_statLine;
 #endif // wxUSE_STATLINE
+        // delete m_sizerStatBox; -- deleted by Remove()
+        m_sizerStatic->Remove(m_sizerStatBox);
     }
 
     int flagsBox = 0,

comment:2 Changed 7 years ago by vadz

  • Component changed from samples to GUI-all
  • Keywords regression added
  • Milestone set to 3.0.1
  • Summary changed from widgets staticbox sample breaks to Removing static box sizer deletes all its contents
  • Version changed from dev-latest to 3.0.0

The problem here is that this code used to work but now doesn't because removing the sizer deletes its associated static box which, in turn, deletes all its children (i.e. m_statText, m_statMarkup and m_statLine). So while fixing it by changing the sample is indeed simple, I'd like to find some way of fixing it in wx itself...

Unfortunately I don't see any really good way to do it, the only thing I can think of is to reparent all the static box children under its parent before deleting it. I think it should fix this bug and, more importantly, preserve the general rule that deleting a sizer doesn't delete any windows managed by it.

Do you see any problems with this solution or any better ones?

comment:3 Changed 7 years ago by vadz

  • Patch set

I'd appreciate if people could please test if the following patch doesn't result in any new problems:

  • src/common/sizer.cpp

    diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp
    index dae2125..d499921 100644
    a b wxStaticBoxSizer::wxStaticBoxSizer(int orient, wxWindow *win, const wxString& s) 
    24492449
    24502450wxStaticBoxSizer::~wxStaticBoxSizer()
    24512451{
    2452     delete m_staticBox;
     2452    // As an exception to the general rule that sizers own other sizers that
     2453    // they contain but not the windows managed by them, this sizer does own
     2454    // the static box associated with it (which is not very logical but
     2455    // convenient in practice and, most importantly, can't be changed any more
     2456    // because of compatibility). However we definitely should not destroy the
     2457    // children of this static box when we're being destroyed, as this would be
     2458    // unexpected and break the existing code which worked with the windows
     2459    // created as siblings of the static box instead of its children in the
     2460    // previous wxWidgets versions, so ensure they are left alive.
     2461
     2462    if ( m_staticBox )
     2463    {
     2464        // Notice that we must make a copy of the list as it will be changed by
     2465        // Reparent() calls in the loop.
     2466        const wxWindowList children = m_staticBox->GetChildren();
     2467        wxWindow* const parent = m_staticBox->GetParent();
     2468        for ( wxWindowList::const_iterator i = children.begin();
     2469              i != children.end();
     2470              ++i )
     2471        {
     2472            (*i)->Reparent(parent);
     2473        }
     2474
     2475        delete m_staticBox;
     2476    }
    24532477}
    24542478
    24552479void wxStaticBoxSizer::RecalcSizes()

TIA!

comment:4 Changed 6 years ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [75921]) Don't delete child controls when deleting wxStaticBoxSizer.

This is an incompatible change compared to 2.8 which can make the existing
code crash and it also goes against the usual rule that the windows are never
owned by sizers, only other windows.

Closes #15698.

comment:5 Changed 6 years ago by VZ

(In [75922]) Don't delete child controls when deleting wxStaticBoxSizer.

This is an incompatible change compared to 2.8 which can make the existing
code crash and it also goes against the usual rule that the windows are never
owned by sizers, only other windows.

Closes #15698.

Note: See TracTickets for help on using tickets.