Question about m_InitDone in ClassBrowserBuilderThread class

[ollydbg]

I see such comments in the end part of the function void ClassBrowserBuilderThread::BuildTree()

Code

    // Initialisation is done after Init() and at least *one* call to BuildTree().
    // Also, in Init() m_InitDone is set to false, directly followed by a
    // re-launch of the thread resulting in a call to BuildTree() due to
    // posting the semaphore from ClassBrowser.
    m_InitDone = true;

    wxCommandEvent e2(wxEVT_COMMAND_ENTER, m_idThreadEvent);
    e2.SetInt(buildTreeEnd);
    m_Parent->AddPendingEvent(e2);
}

Question: I don't understand the comments, I see that m_InitDone never set to false in Init(), can you explain?
What does the m_InitDone usd for?

BTW:
I see that some times, the function

Code

    // initialise it
    m_ClassBrowserBuilderThread->Init(m_NativeParser,
                                      m_CCTreeCtrl,
                                      m_CCTreeCtrlBottom,
                                      m_ActiveFilename,
                                      activeProject,
                                      m_Parser->ClassBrowserOptions(),
                                      m_Parser->GetTokenTree(),
                                      idThreadEvent);

Is directly called from main GUI thread, at this time, the worker thread is forced in paused status now, but does that cause some multiply thread issue?

BTW2:
Since the tree builder thread can send buildTreeEnd event to gui main thread's classbrowser class, can classbrowser  class remember some status of the build thread, and do any thing? Currently, the event receiver only print some debug log message.

[ollydbg]

I see such comments in the end part of the function void ClassBrowserBuilderThread::BuildTree()

Code

    // Initialisation is done after Init() and at least *one* call to BuildTree().
    // Also, in Init() m_InitDone is set to false, directly followed by a
    // re-launch of the thread resulting in a call to BuildTree() due to
    // posting the semaphore from ClassBrowser.
    m_InitDone = true;

    wxCommandEvent e2(wxEVT_COMMAND_ENTER, m_idThreadEvent);
    e2.SetInt(buildTreeEnd);
    m_Parent->AddPendingEvent(e2);
}

Question: I don't understand the comments, I see that m_InitDone never set to false in Init(), can you explain?
What does the m_InitDone usd for?

Today, when reviewing the code, I see this issue again. For the comments above, I think I will set the  m_InitDone to false at the beginning of the function:

Code

void ClassBrowserBuilderThread::Init(NativeParser*         np,
                                     CCTreeCtrl*           treeTop,
                                     CCTreeCtrl*           treeBottom,
                                     const wxString&       active_filename,
                                     void*                 user_data, // active project
                                     const BrowserOptions& bo,
                                     TokenTree*            tt,
                                     int                   idThreadEvent)
{
    TRACE(_T("ClassBrowserBuilderThread::Init"));

    CC_LOCKER_TRACK_CBBT_MTX_LOCK(m_ClassBrowserBuilderThreadMutex);

    // this indicates the tree becomes invalid, because after the Init() function, the tree will be
    // rebuild
    m_InitDone = false; // add this line here.

[ollydbg]

Well, things are not that simple:

Code

// Functions accessible from outside

void ClassBrowserBuilderThread::ExpandItem(wxTreeItemId item)
{
    TRACE(_T("ClassBrowserBuilderThread::ExpandItem"));

    if (CBBT_SANITY_CHECK || !item.IsOk())
        return;

    bool locked = false;
    if (m_InitDone)
    {
        CC_LOCKER_TRACK_CBBT_MTX_LOCK(m_ClassBrowserBuilderThreadMutex)
        locked = true;
    }
    ...

What about if "m_InitDone" is false, then we don't need that locker?

[ollydbg]

I see more issues about the tree.
When the symbol is initially building:
This is triggered by the function:

Code

void NativeParser::CreateClassBrowser()
{
    ...
    m_ClassBrowser->SetParser(m_Parser); // Also updates class browser
    ...
}

The SetParser actually call the ClassBrowser::UpdateClassBrowserView(), and ClassBrowser::ThreadedBuildTree(), in this function, a worker thread is created, and when the GUI thread(ClassBrowser) release the m_ClassBrowserSemaphore(by calling m_ClassBrowserSemaphore.Post(), the worker thread get the semephore resource, and try to build the tree.

One step to build tree is in:

Code

void ClassBrowserBuilderThread::BuildTree()
{
    ...
    // 9.) Expand item --> Bottleneck: Takes ~4 secs on C::B workspace
    m_CCTreeCtrlTop->Expand(root);

See the backtrace below:

Code

#0  0x7c90e514 in ntdll!KiFastSystemCallRet () from C:\WINDOWS\system32\ntdll.dll
#1  0x7e4194be in USER32!GetWindowLongA () from C:\WINDOWS\system32\user32.dll
#2  0x7e42af42 in USER32!GetDlgCtrlID () from C:\WINDOWS\system32\user32.dll
#3  0x7e4292e3 in USER32!SendMessageW () from C:\WINDOWS\system32\user32.dll
#4  0x011c1653 in wxTreeCtrl::DoExpand(wxTreeItemId const&, int) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#5  0x011c1739 in wxTreeCtrl::Expand(wxTreeItemId const&) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#6  0x66090fc6 in ClassBrowserBuilderThread::BuildTree (this=0x3bc7558) at F:\cb_sf_git\trunk\src\plugins\codecompletion\classbrowserbuilderthread.cpp:516
#7  0x6608f981 in ClassBrowserBuilderThread::Entry (this=0x3bc7558) at F:\cb_sf_git\trunk\src\plugins\codecompletion\classbrowserbuilderthread.cpp:224
#8  0x01144ba9 in wxThreadInternal::DoThreadStart(wxThread*) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#9  0x01144c85 in wxThreadInternal::WinThreadStart(void*)@4 () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#10 0x77c3a3b0 in msvcrt!_endthreadex () from C:\WINDOWS\system32\msvcrt.dll
#11 0x7c80b729 in KERNEL32!GetModuleFileNameA () from C:\WINDOWS\system32\kernel32.dll
#12 0x00000000 in ?? ()

But when the root node is expanded , I see that the GUI thread's event handler get called:

Code

// Functions accessible from outside

void ClassBrowserBuilderThread::ExpandItem(wxTreeItemId item)
{
    TRACE(_T("ClassBrowserBuilderThread::ExpandItem"));

    if (CBBT_SANITY_CHECK || !item.IsOk())
        return;

    bool locked = false;
    if (m_InitDone)
    {
        CC_LOCKER_TRACK_CBBT_MTX_LOCK(m_ClassBrowserBuilderThreadMutex)
        locked = true;
    }

#ifdef CC_BUILDTREE_MEASURING
    wxStopWatch sw;
#endif

    CC_LOCKER_TRACK_TT_MTX_LOCK(s_TokenTreeMutex)

    CCTreeCtrlData* data = static_cast<CCTreeCtrlData*>(m_CCTreeCtrlTop->GetItemData(item));
    if (data)
        m_TokenTree->RecalcInheritanceChain(data->m_Token);

    CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)

    if (data)
    {
        switch (data->m_SpecialFolder)
        {
            case sfRoot:
            {
                CreateSpecialFolders(m_CCTreeCtrlTop, item);
                if( !(   m_BrowserOptions.displayFilter == bdfFile
                      && m_ActiveFilename.IsEmpty() ) )
                    AddChildrenOf(m_CCTreeCtrlTop, item, -1, ~(tkFunction | tkVariable | tkMacroDef | tkTypedef | tkMacroUse));
                break;
            }
...

Please note that in the event handler, it try to operate on the symbol tree, see the above code, this is done in the main gui thread. But while the worker thread is still running, it will continue to run those code which also operate on the symbol tree.

Code

    // 9.) Expand item --> Bottleneck: Takes ~4 secs on C::B workspace
    m_CCTreeCtrlTop->Expand(root);
#ifdef CC_BUILDTREE_MEASURING
    CCLogger::Get()->DebugLog(F(_T("Expanding root item took : %ld ms"),sw.Time()));
    sw.Start();
#endif
#endif // CC_NO_COLLAPSE_ITEM

    // seems like the "expand" event comes too late in wxGTK, so make it happen now
    if (platform::gtk || platform::macosx)
        ExpandItem(root);
#ifdef CC_BUILDTREE_MEASURING
    CCLogger::Get()->DebugLog(F(_T("Expanding root item (gtk only) took : %ld ms"),sw.Time()));
    sw.Start();
#endif

    // 10.) Expand the items saved before
    ExpandSavedItems(m_CCTreeCtrlTop, root, 0);
    ...
}

So, what's the hell is that both the GUI thread and the worker thread are now operating on the symbol tree. 

Look at the code above, you see that:

Code

    // seems like the "expand" event comes too late in wxGTK, so make it happen now
    if (platform::gtk || platform::macosx)
        ExpandItem(root);

What does this means, this means that under wxGTK, the function: void ClassBrowserBuilderThread::ExpandItem(wxTreeItemId item) is explicitly called(that's OK, it is in the worker thread), but it looks like under Windows, the GUI thread's event handler also call this function(that's BAD, because it is not in the worker thread, but in the GUI thread)

The event handler:

Code

void ClassBrowser::OnTreeItemExpanding(wxTreeEvent& event)
{
    if (m_ClassBrowserBuilderThread)
        m_ClassBrowserBuilderThread->ExpandItem(event.GetItem());
#ifndef CC_NO_COLLAPSE_ITEM
    event.Allow();
#endif // CC_NO_COLLAPSE_ITEM
}

This is the backtrace of the main GUI thread:

Code

#0  ClassBrowserBuilderThread::ExpandItem (this=0x3bc7558, item=...) at F:\cb_sf_git\trunk\src\plugins\codecompletion\classbrowserbuilderthread.cpp:254
#1  0x6608d8d1 in ClassBrowser::OnTreeItemExpanding (this=0x3c61bb0, event=...) at F:\cb_sf_git\trunk\src\plugins\codecompletion\classbrowser.cpp:945
#2  0x010e15f1 in wxAppConsole::HandleEvent(wxEvtHandler*, void (wxEvtHandler::*)(wxEvent&), wxEvent&) const () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#3  0x0114a07e in wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#4  0x0114a14a in wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#5  0x0114a545 in wxEvtHandler::ProcessEvent(wxEvent&) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#6  0x0121f502 in wxWindowBase::TryParent(wxEvent&) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#7  0x0121f502 in wxWindowBase::TryParent(wxEvent&) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#8  0x0121f502 in wxWindowBase::TryParent(wxEvent&) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#9  0x011c1e27 in wxTreeCtrl::MSWOnNotify(int, long, long*) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#10 0x0118616c in wxWindow::MSWWindowProc(unsigned int, unsigned int, long) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#11 0x01180cee in wxWndProc(HWND__*, unsigned int, unsigned int, long)@16 () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#12 0x7e418734 in USER32!GetDC () from C:\WINDOWS\system32\user32.dll
#13 0x00120822 in ?? ()
#14 0x0000004e in ?? ()
#15 0x00000589 in ?? ()
#16 0x0022e730 in ?? ()
#17 0x7e418816 in USER32!GetDC () from C:\WINDOWS\system32\user32.dll
#18 0x01180ca0 in wxWindow::AssociateHandle(void*) () from E:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
#19 0x7e42927b in USER32!GetParent () from C:\WINDOWS\system32\user32.dll
#20 0x00000000 in ?? ()

Too bad as I can see about the operation on symbol tree operation.

[ollydbg]

I see such comments in the end part of the function void ClassBrowserBuilderThread::BuildTree()

Code

    // Initialisation is done after Init() and at least *one* call to BuildTree().
    // Also, in Init() m_InitDone is set to false, directly followed by a
    // re-launch of the thread resulting in a call to BuildTree() due to
    // posting the semaphore from ClassBrowser.
    m_InitDone = true;

    wxCommandEvent e2(wxEVT_COMMAND_ENTER, m_idThreadEvent);
    e2.SetInt(buildTreeEnd);
    m_Parent->AddPendingEvent(e2);
}

Question: I don't understand the comments, I see that m_InitDone never set to false in Init(), can you explain?
What does the m_InitDone usd for?

I see that rev7778 add this code snippet, but is the comment wrong?, I looked at all the changs in rev7778, I don't see any code which set m_InitDone to false.

As m_InitDone is true, than most function of ClassBrowserBuilderThread, such as void ClassBrowserBuilderThread::ExpandItem(wxTreeItemId item) won't be called parallel, because it has a locker:

Code

void ClassBrowserBuilderThread::ExpandItem(wxTreeItemId item)
{
    TRACE(_T("ClassBrowserBuilderThread::ExpandItem"));

    if (CBBT_SANITY_CHECK || !item.IsOk())
        return;

    bool locked = false;
    if (m_InitDone)
    {
        CC_LOCKER_TRACK_CBBT_MTX_LOCK(m_ClassBrowserBuilderThreadMutex)
        locked = true;
    }
    ...