Crash on start (wxWidgets 3.3.1, 64 bits)

[Miguel Gimenez]

I have compiled C::B with the "official" Bretch Sander's compiler, and get a consistent crash on start.

Code

Thread 1 received signal SIGSEGV, Segmentation fault.

#0  0x000007fee574e40f in wxBitmapBundle::operator=(wxBitmapBundle const&) () from C:\Windows\system32\wxmsw331u_gcc_custom.dll
#1  0x000007fee5866564 in __gnu_cxx::__normal_iterator<wxAuiNotebookPage*, std::vector<wxAuiNotebookPage, std::allocator<wxAuiNotebo
okPage> > > std::_V2::__rotate<__gnu_cxx::__normal_iterator<wxAuiNotebookPage*, std::vector<wxAuiNotebookPage, std::allocator<wxAuiN
otebookPage> > > >(__gnu_cxx::__normal_iterator<wxAuiNotebookPage*, std::vector<wxAuiNotebookPage, std::allocator<wxAuiNotebookPage>
 > >, __gnu_cxx::__normal_iterator<wxAuiNotebookPage*, std::vector<wxAuiNotebookPage, std::allocator<wxAuiNotebookPage> > >, __gnu_c
xx::__normal_iterator<wxAuiNotebookPage*, std::vector<wxAuiNotebookPage, std::allocator<wxAuiNotebookPage> > >, std::random_access_i
terator_tag) [clone .isra.0] () from C:\Windows\system32\wxmsw331u_gcc_custom.dll
#2  0x000007fee5867a13 in wxAuiTabContainer::MovePage(unsigned long long, unsigned long long) ()
   from C:\Windows\system32\wxmsw331u_gcc_custom.dll
#3  0x000007fed4618868 in cbAuiNotebook::MovePage (this=0x359e6a0, page=0x8379470, new_idx=7)
    at G:\codeblocks\src\sdk\cbauibook.cpp:498
#4  0x000000013f15742b in InfoPane::ReorderTabs (this=0x359e6a0,
    cmp_f=0x13f1571e0 <InfoPane::CompareIndexes(InfoPane::Page**, InfoPane::Page**)>) at G:\codeblocks\src\src\infopane.cpp:153
#5  0x000000013f1570f7 in InfoPane::LoadTabOrder (this=0x359e6a0, layout=...) at G:\codeblocks\src\src\infopane.cpp:119
#6  0x000000013f164575 in MainFrame::LoadViewLayout (this=0x3445220, name=..., isTemp=false) at G:\codeblocks\src\src\main.cpp:1862
#7  0x000000013f163121 in MainFrame::LoadWindowState (this=0x3445220) at G:\codeblocks\src\src\main.cpp:1717
#8  0x000000013f15ab34 in MainFrame::MainFrame (this=0x3445220, parent=0x0) at G:\codeblocks\src\src\main.cpp:821
#9  0x000000013f103456 in CodeBlocksApp::InitFrame (this=0x366690) at G:\codeblocks\src\src\app.cpp:523
#10 0x000000013f104dd2 in CodeBlocksApp::OnInit (this=0x366690) at G:\codeblocks\src\src\app.cpp:754
#11 0x000000013f1ff3b5 in wxAppConsoleBase::CallOnInit (this=0x366690) at C:/Librerias151/wxWidgets-3.3.1/include/wx/app.h:92
#12 0x000007fee529a007 in wxEntryReal(int&, wchar_t**) () from C:\Windows\system32\wxmsw331u_gcc_custom.dll
#13 0x000000013f102721 in WinMain (hInstance=0x13f100000, hPrevInstance=0x0, lpCmdLine=0x28428e "", nCmdShow=10)
    at G:\codeblocks\src\src\app.cpp:334
#14 0x000000013f1dd364 in main ()

I have isolated the problem to this code:

Code

void InfoPane::ReorderTabs(CompareFunction cmp_f)
{
    if (m_Pages.GetCount() == 0)
        return;
    m_Pages.Sort(cmp_f);

    cbAuiNotebook::Hide();
    int index = 0;
    for (size_t i = 0 ; i < m_Pages.GetCount(); ++i)
    {
        int pageIndex = GetPageIndex(m_Pages.Item(i)->window);
        if (m_Pages.Item(i)->indexInNB < 0)
        {
            if (pageIndex >= 0)
                RemovePage(pageIndex);
            if (m_Pages.Item(i)->window)
                m_Pages.Item(i)->window->Hide();
        }
        else
        {
            if (pageIndex < 0)
                AddPagePrivate(m_Pages.Item(i)->window, m_Pages.Item(i)->title, m_Pages.Item(i)->icon);
            if (index++ != pageIndex)
                MovePage(m_Pages.Item(i)->window, index );   <------- HERE
        }
    }
    cbAuiNotebook::Show();
}

where in the call to MovePage() index is equal to the number of pages in the notebook, so the call to std::rotate uses an invalid iterator.

I have tried to fix the logic, but only got a corruption of the layout when closing C::B. A check for index < GetPageCount() works, but IMHO fixing the logic is better.

Failed attempt:

Code

            if (index != pageIndex)
                MovePage(m_Pages.Item(i)->window, index);
            index++;

Working but undesired attempt:

Code

            if (index++ != pageIndex)
                if (index < (int)GetPageCount())
                    MovePage(m_Pages.Item(i)->window, index);

Any ideas?

EDIT: The code in wxAuiTabContainer::MovePage() has been completely changed from wx3.2.8 to wx3.3.1, now uses std::rotate()

[blauzahn]

Does it crash as well in --safe-mode or when you remove the zip-files of plugins manually one by one before start?

I recently observe frequent crashes as well with tabs, especially on changes. Cb also started to hang often at shutdown.

my 2 cents about the code-snippet:

The second arg to MovePage is of unsigned type size_t, not int. See cbauibook.h:107:

Code

 
* Moves the tab containing page to new_idx
* \param page The page to move (e.g. cbEditor*)
* \param new_idx The index the page should be moved to
* \return true if successful
*/
        bool MovePage(wxWindow* page, size_t new_idx);

• The return values of the functions RemovePage, AddPagePrivate and MovePage are not checked.
• What happens to the indices when RemovePage is called?
• pageIndex can be made const.
• GetPageIndex returns wxNOT_FOUND if the window is not found in the notebook. Comparing pageIndex < 0 therefore looks like bad style, even if wxNOT_FOUND is -1 and will probably never change.
• Is that the same with indexInNB < 0?
• Does the CompareFunction passed in define strict weak ordering?
• After m_Pages.Sort the member-variable m_pages it looks like it is not modified until the end of the function. Having a const& to it would make that clear.
• Within the loop the same m_Pages.Item(i) is used several times. Again, having a const& to it would make the logic more readable.
• Is it correct to post-fix incremented index++ even when a call to MovePage will not be done? If so, I'd pull it out of the if statement like you did in your failed attempt.
• Is is guaranteed, that ->window is never a nullptr when ->Hide is called on it?
• m_Pages.GetCount() == 0 should be replaced by m_Pages.IsEmpty().
• The code is not exception safe. When an exception gets thrown after the Hide, it will not reach Show and therefore remain hidden.

Sorry that I can not provide better ideas due to my total lack of knowledge in wxWidgets. Maybe these things trigger another idea from you despite that.

[PB]

Just a blind shot:

I would try to rebuild with wxWidgets 3.2 to see if the problem persists.

If it works there, perhaps this is relevant:

The meaning of page index in wxAuiNotebook has changed if the pages have
  been reordered (see wxAUI_NB_TAB_MOVE) and now always refers to the page
  logical index, which is not affected by reordering. To get the position of
  the page on screen, which doesn't make sense without reference to the tab
  control containing it, use GetPagePosition() to retrieve both of them.

From https://github.com/wxWidgets/wxWidgets/blob/d557e926b187afe29e37847ba6652da326e1bba7/docs/changes.txt#L135C3-L139C73

[blauzahn]

I get an assert on startup:

../trunk/src/aui/auibook.cpp(2857): assert "Assert failure" failed in FindTab(): Window unexpectedly not found in any tab control

In safe-mode it does not show up. Its svn build  rev 13693.

I have not yet compiled the latest svn with wx-3.2.

[Miguel Gimenez]

Thank you both for the comments. I have made some changes

Code

void InfoPane::ReorderTabs(CompareFunction cmp_f)
{
    if (m_Pages.IsEmpty())
        return;

    m_Pages.Sort(cmp_f);

    cbAuiNotebook::Hide();

    size_t index = 0;
    for (size_t i = 0 ; i < m_Pages.GetCount(); ++i)
    {
        const Page* page = m_Pages.Item(i);
        int pageIndex = GetPageIndex(page->window);  // wx3.3.0 has GetPagePosition()
        if (page->indexInNB < 0)
        {
            if (pageIndex != wxNOT_FOUND)
                RemovePage(pageIndex);

            if (page->window)
                page->window->Hide();
        }
        else
        {
            if (pageIndex == wxNOT_FOUND)
                pageIndex = AddPagePrivate(page->window, page->title, page->icon);

            if (index++ != static_cast <size_t> (pageIndex))
                if (index < GetPageCount())
                    MovePage(page->window, index);
        }
    }

    cbAuiNotebook::Show();
}

that work with wxWidgets 3.3.1, but if you later start the "old" C::B (compiled with wx3.2.8 ) the layout in default.conf is corrupted.

I have tested with GetPagePosition() but there is no difference.

I think the problem is MovePage() in wx3.2.8 is tolerant to invalid indexes, and the behaviour with these invalid indexes is needed in the algorithm.

EDIT: IIRC saving AUI layouts has also changed, I hope this is not related.

[Miguel Gimenez]

I have found the origin of the "corruption": wxWidgets 3.3 saves perspectives in layout3 format, while wxWidgets 3.2 only recognizes layout1 and layout2. This comment in wxAuiManager::LoadPerspective() explains the scope of each format:

Code

  // check layout string version
    //    'layout1' = wxAUI 0.9.0 - wxAUI 0.9.2
    //    'layout2' = wxAUI 0.9.2 (wxWidgets 2.8)
    //    'layout3' = wxWidgets 3.3.1

Sadly, there is no way to make SavePerspective() use layout2 format, so when switching from C::B compiled with wx3.3 to another compiled with wx3.2 the saved perspective will be ignored silently. The inverse will work OK.

This is independent of my proposed patch, so I think I can apply it.