codecompletion plugin lead codeblocks crash, do GUI operation in worker thread

[kipade]

I am using codeblocks under linux, I have tested under several distributions, but got the same result.
CodeBlocks always crash during working, due to codecompletion plugin.
I have had a look on its source code, I want to know why it must do GUI operation in non-main thread, but in worker thread, which cause crashing.

[ollydbg]

I am using codeblocks under linux, I have tested under several distributions, but got the same result.
CodeBlocks always crash during working, due to codecompletion plugin.

Me too, but it is really hard to catch the bug, I even don't have ways to reproduce some hang bug. Sometimes, it works, sometimes, it hangs.

I have had a look on its source code, I want to know why it must do GUI operation in non-main thread, but in worker thread, which cause crashing.

In CC, the symbol browser trees (wxTreeCtrl) were built from a thread class named ClassBrowserBuilderThread. The reason I think is that the tree can have many labels(Tokens), and it take several seconds to build the tree, so the devs put those operations in a worker thread. This was done years before I becomes a C::B developer.

Do you think there are some better solution?

BTW: I'm not sure which code cause the crash, maybe it is not the GUI operation in non-main thread. CC code contains a lot of mutex lockers, I hate them, but I also don't know how to fix them. So, comments/solutions/patches are all welcome.

[kipade]

hello ollydbg, thanks for your reply. As you said, it crashes mostly at BuildTree here, which do some UI operations.
And, I have no further ideas about that before I have a deep look on CB's source code. But, its a common method to do UI operations in worker thread is via notifications, right?
Mightbe, the operations and data should be separated, for this condition.

[ollydbg]

...But, its a common method to do UI operations in worker thread is via notifications, right?

Basically, all the UI operations should be put in the main thread, note the bottle neck is building the big wxTreeCtrl, so notification method is not involved here.

Mightbe, the operations and data should be separated, for this condition.

Yes, they should be separated, but if you want to construct a big wxTreeCtrl, what is a good method?

[kipade]

Mightbe a big TreeCtrl was not necessary, a ugly way is to define a custom event to do such UI operations, just use custom data and callback method, I do not think its impossiable.

[kipade]

Hello ollydbg, I just define a simple event to notify ClassBrowser to call ClassBrowserBuilderThread's BuildTree function, because BuildTree was originally called in a while loop, also has semaphore to guarantee ths order of the operations. So, it works for me, under linux. Although this is not a good idea, it really works. And, this also has not make the GUI busy, almost fell nothing of it.

[ollydbg]

Hello ollydbg, I just define a simple event to notify ClassBrowser to call ClassBrowserBuilderThread's BuildTree function, because BuildTree was originally called in a while loop, also has semaphore to guarantee ths order of the operations. So, it works for me, under linux. Although this is not a good idea, it really works. And, this also has not make the GUI busy, almost fell nothing of it.

Thanks, can you show some patches? So that I can tested on my local PC(winXP).

[oBFusCATed]

Probably the best way is to define a custom event where you can store 50 or 100 items. Then in the worker thread you just send these elements and in the main thread you add the items to the tree in small batches. I think this won't block the ui and user events might be process normally.

[kipade]

Probably the best way is to define a custom event where you can store 50 or 100 items. Then in the worker thread you just send these elements and in the main thread you add the items to the tree in small batches. I think this won't block the ui and user events might be process normally.

Mightbe. But, it need more coding work, right? I just define a custom event and simply move the BuildTree into ClassBrowser window messages processing chain. I also think this operation does need to be processed in this window class, since, all operations of this function is work for ClassBrowser. All of this GUI operations in this function also would not take too much time. If move the responsibility to ClassBrowser, the function should be rewrite, and, some class definition should be redesined,too. I think.

[oBFusCATed]

kipade: Have you do some work on this or this is just a suggestion?

[kipade]

Hello ollydbg, I just define a simple event to notify ClassBrowser to call ClassBrowserBuilderThread's BuildTree function, because BuildTree was originally called in a while loop, also has semaphore to guarantee ths order of the operations. So, it works for me, under linux. Although this is not a good idea, it really works. And, this also has not make the GUI busy, almost fell nothing of it.

Thanks, can you show some patches? So that I can tested on my local PC(winXP).

I packed my modified files, I do not use patch file because Im afraid my base code was outdated.  my base code was svn9925
This code is just a try, a joke, lol.

[kipade]

kipade: Have you do some work on this or this is just a suggestion?

This just a try, of course I want the code looks perfect and works well.

[ollydbg]

Thanks kipade, this is the patch against git-svn head, but I have build errors.

Code

196d6a6c37e1ff7f91bd76cbafd62d645c1f2e03
 src/plugins/codecompletion/BuildTreeEvent.h        | 37 ++++++++++++++++++++++
 src/plugins/codecompletion/classbrowser.cpp        | 11 +++++++
 src/plugins/codecompletion/classbrowser.h          |  3 ++
 .../codecompletion/classbrowserbuilderthread.cpp   | 11 +++++--
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/src/plugins/codecompletion/BuildTreeEvent.h b/src/plugins/codecompletion/BuildTreeEvent.h
new file mode 100644
index 0000000..73cf2c5
--- /dev/null
+++ b/src/plugins/codecompletion/BuildTreeEvent.h
@@ -0,0 +1,37 @@
+#ifndef __BUILD_TREE_H__
+#define __BUILD_TREE_H__
+
+/*
+ * Simple BuildTree event for ClassBrowser
+ *
+ * Author: ZhiYu
+ */
+
+#include <wx/event.h>
+#include "classbrowserbuilderthread.h"
+
+class cbBuildTreeEvent;
+wxDECLARE_EVENT(cbBUILD_CLASSTREE_EVENT,cbBuildTreeEvent);
+class cbBuildTreeEvent : public wxCommandEvent
+{
+public:
+  cbBuildTreeEvent(ClassBrowserBuilderThread* builderThreadObject,wxEventType cmdType=cbBUILD_CLASSTREE_EVENT):wxCommandEvent(cmdType),clsBuilderThread(builderThreadObject){
+  }
+
+  ClassBrowserBuilderThread* GetBuilderThreadObject() const { return clsBuilderThread; }
+  cbBuildTreeEvent* Clone()
+  {
+    return new cbBuildTreeEvent(*this);
+  }
+private:
+  ClassBrowserBuilderThread* clsBuilderThread;
+};
+
+//following lines are only for you if you did not like use event table
+typedef void (wxEvtHandler::*cbBuildTreeFunction)(cbBuildTreeEvent&);
+
+#define cbBuildTreeEventHandler(func) wxEVENT_HANDLER_CAST(cbBuildTreeFunction,func)
+#define EVT_CBBUILD_CLASSTREE(id,func) \
+  wx__DECLARE_EVT1(cbBUILD_CLASSTREE_EVENT,id,cbBuildTreeEventHandler(func))
+
+#endif
diff --git a/src/plugins/codecompletion/classbrowser.cpp b/src/plugins/codecompletion/classbrowser.cpp
index cd96467..4914d86 100644
--- a/src/plugins/codecompletion/classbrowser.cpp
+++ b/src/plugins/codecompletion/classbrowser.cpp
@@ -44,6 +44,7 @@
 #include "nativeparser.h"
 
 #include "parser/ccdebuginfo.h"
+#include <stdio.h>
 
 #define CC_CLASS_BROWSER_DEBUG_OUTPUT 0
 
@@ -76,6 +77,8 @@
     #define TRACE2(format, args...)
 #endif
 
+wxDEFINE_EVENT(cbBUILD_CLASSTREE_EVENT,cbBuildTreeEvent);
+
 int idMenuJumpToDeclaration    = wxNewId();
 int idMenuJumpToImplementation = wxNewId();
 int idMenuRefreshTree          = wxNewId();
@@ -127,6 +130,7 @@ BEGIN_EVENT_TABLE(ClassBrowser, wxPanel)
     EVT_MENU(idCBBottomTree,                             ClassBrowser::OnCBViewMode)
 
     EVT_COMMAND(idThreadEvent, wxEVT_COMMAND_ENTER,      ClassBrowser::OnThreadEvent)
+    EVT_CBBUILD_CLASSTREE(wxID_ANY,ClassBrowser::OnBuileTree)
 END_EVENT_TABLE()
 
 // class constructor
@@ -971,3 +975,10 @@ void ClassBrowser::OnThreadEvent(wxCommandEvent& event)
             break;
     }
 }
+
+void ClassBrowser::OnBuileTree(cbBuildTreeEvent& event)
+{
+  ClassBrowserBuilderThread* thread = event.GetBuilderThreadObject();
+  if(thread)
+    thread->BuildTree();
+}
diff --git a/src/plugins/codecompletion/classbrowser.h b/src/plugins/codecompletion/classbrowser.h
index 9715c07..1a272c7 100644
--- a/src/plugins/codecompletion/classbrowser.h
+++ b/src/plugins/codecompletion/classbrowser.h
@@ -16,6 +16,7 @@
 #include "classbrowserbuilderthread.h"
 #include "parser/parser.h"
 #include "parser/token.h"
+#include "BuildTreeEvent.h"
 
 class NativeParser;
 class wxComboBox;
@@ -90,6 +91,8 @@ private:
     void OnDebugSmartSense(wxCommandEvent& event);
     void OnSetSortType(wxCommandEvent& event);
 
+    void OnBuileTree(cbBuildTreeEvent& event);
+
     void OnSearch(wxCommandEvent& event);
     bool FoundMatch(const wxString& search, wxTreeCtrl* tree, const wxTreeItemId& item);
     wxTreeItemId FindNext(const wxString& search, wxTreeCtrl* tree, const wxTreeItemId& start);
diff --git a/src/plugins/codecompletion/classbrowserbuilderthread.cpp b/src/plugins/codecompletion/classbrowserbuilderthread.cpp
index 24f05cb..6640ef0 100644
--- a/src/plugins/codecompletion/classbrowserbuilderthread.cpp
+++ b/src/plugins/codecompletion/classbrowserbuilderthread.cpp
@@ -63,6 +63,8 @@
     #define TRACE2(format, args...)
 #endif
 
+#include "BuildTreeEvent.h"
+
 ClassBrowserBuilderThread::ClassBrowserBuilderThread(wxEvtHandler* evtHandler, wxSemaphore& sem) :
     wxThread(wxTHREAD_JOINABLE),
     m_Parent(evtHandler),
@@ -184,7 +186,7 @@ void ClassBrowserBuilderThread::Init(NativeParser*         np,
 }
 
 // Thread function
-
+#include <stdio.h>
 void* ClassBrowserBuilderThread::Entry()
 {
     while (!m_TerminationRequested && !Manager::IsAppShuttingDown() )
@@ -206,7 +208,10 @@ void* ClassBrowserBuilderThread::Entry()
                 ::wxMutexGuiEnter();
         }
 
-        BuildTree();
+        //BuildTree();
+        cbBuildTreeEvent* evt = new cbBuildTreeEvent(this,cbBUILD_CLASSTREE_EVENT);
+        if(m_Parent)
+          m_Parent->QueueEvent(evt);
 
         if (platform::gtk || platform::macosx)
         {
@@ -447,6 +452,8 @@ void ClassBrowserBuilderThread::BuildTree()
     // 6.) Hide&Freeze trees shown
     if (m_BrowserOptions.treeMembers)
     {
+        if(m_CCTreeCtrlBottom==NULL)
+          wxMessageBox(wxT("kcynice:classbrowserbuilderthread.cpp::BuildTree m_CCTreeCtrlBottom is null"));
         m_CCTreeCtrlBottom->Hide();
         m_CCTreeCtrlBottom->Freeze();
     }

Build error wx2.8.12, winXP.

Code

[ 20.0%] g++.exe -Wall -g -pipe -fmessage-length=0 -fexceptions -mthreads -Winvalid-pch -DHAVE_W32API_H -D__WXMSW__ -DWXUSINGDLL -DcbDEBUG -DCB_PRECOMP -DWX_PRECOMP -DwxUSE_UNICODE -DEXPORT_LIB=0 -DEXPORT_EVENTS=0 -DwxUSE_WX_RESOURCES=0 -DBUILDING_PLUGIN -IE:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\include -IE:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\mswu -I..\..\include -I..\..\sdk\wxscintilla\include -I..\..\include\mozilla_chardet -I..\..\include\mozilla_chardet\mfbt -I..\..\include\mozilla_chardet\nsprpub\pr\include -I..\..\include\mozilla_chardet\xpcom -I..\..\include\mozilla_chardet\xpcom\base -I..\..\include\mozilla_chardet\xpcom\glue -c classbrowser.cpp -o ..\..\.objs\plugins\codecompletion\classbrowser.o
In file included from classbrowser.h:19:0,
                 from classbrowser.cpp:43:
BuildTreeEvent.h:14:16: error: expected constructor, destructor, or type conversion before '(' token
BuildTreeEvent.h:18:87: error: 'cbBUILD_CLASSTREE_EVENT' was not declared in this scope
classbrowser.cpp:80:15: error: expected constructor, destructor, or type conversion before '(' token
classbrowser.cpp:133:5: error: 'cbBUILD_CLASSTREE_EVENT' was not declared in this scope
In file included from classbrowser.h:16:0,
                 from classbrowser.cpp:43:
classbrowserbuilderthread.h: In member function 'void ClassBrowser::OnBuileTree(cbBuildTreeEvent&)':
classbrowserbuilderthread.h:64:10: error: 'void ClassBrowserBuilderThread::BuildTree()' is protected
classbrowser.cpp:983:23: error: within this context
Process terminated with status 1 (0 minute(s), 13 second(s))
6 error(s), 0 warning(s) (0 minute(s), 13 second(s))

[kipade]

Thanks kipade, this is the patch against git-svn head, but I have build errors.

I am sorry that, I forgot upload one file, but with only one line added, its in classbrowserbuilderthread.h, add ClassBrowser as a friend class of ClassBrowserBuilderThread. btw, you can do some slight modification to make it work. 

[ollydbg]

A working patch (fix some bugs in kipade' original patch), it seems to work, I don't see time lags (browser tree even set to show everything in a sample wx3.0 prj).

Code

 src/plugins/codecompletion/BuildTreeEvent.h        | 66 ++++++++++++++++++++++
 src/plugins/codecompletion/classbrowser.cpp        | 18 ++++++
 src/plugins/codecompletion/classbrowser.h          |  3 +
 .../codecompletion/classbrowserbuilderthread.cpp   | 14 ++++-
 .../codecompletion/classbrowserbuilderthread.h     |  1 +
 5 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/src/plugins/codecompletion/BuildTreeEvent.h b/src/plugins/codecompletion/BuildTreeEvent.h
new file mode 100644
index 0000000..73d0b1a
--- /dev/null
+++ b/src/plugins/codecompletion/BuildTreeEvent.h
@@ -0,0 +1,66 @@
+#ifndef __BUILD_TREE_H__
+#define __BUILD_TREE_H__
+
+/*
+ * Simple BuildTree event for ClassBrowser
+ *
+ * Author: ZhiYu
+ */
+
+#include <wx/event.h>
+#include "classbrowserbuilderthread.h"
+
+extern const wxEventType cbBUILD_CLASSTREE_EVENT;
+// the below does not work, see:
+// http://permalink.gmane.org/gmane.comp.lib.wxwindows.general/62431
+//DECLARE_EVENT_TYPE(cbBUILD_CLASSTREE_EVENT, wxID_ANY)
+
+class cbBuildTreeEvent : public wxCommandEvent
+{
+public:
+    cbBuildTreeEvent(ClassBrowserBuilderThread* builderThreadObject = NULL,
+                     wxEventType cmdType = cbBUILD_CLASSTREE_EVENT)
+        :wxCommandEvent(cmdType),
+         clsBuilderThread(builderThreadObject)
+    {
+    }
+
+    cbBuildTreeEvent(const cbBuildTreeEvent& rhs)
+        : wxCommandEvent(rhs),
+          clsBuilderThread(rhs.clsBuilderThread)
+
+    {
+    }
+
+    ClassBrowserBuilderThread* GetBuilderThreadObject() const
+    {
+        return clsBuilderThread;
+    }
+    virtual wxEvent *Clone() const
+    {
+        return new cbBuildTreeEvent(*this);
+    }
+//private:
+    ClassBrowserBuilderThread* clsBuilderThread;
+    DECLARE_DYNAMIC_CLASS( cbBuildTreeEvent )
+};
+
+//following lines are only for you if you did not like use event table
+
+typedef void (wxEvtHandler::*cbBuildTreeFunction)(cbBuildTreeEvent&);
+
+
+#define cbBuildTreeEventHandler(func)  \
+	(wxObjectEventFunction)(wxEventFunction) \
+	wxStaticCastEvent(cbBuildTreeFunction, &func)
+
+
+
+#define EVT_CBBUILD_CLASSTREE(id, fn) \
+    DECLARE_EVENT_TABLE_ENTRY(cbBUILD_CLASSTREE_EVENT, id, wxID_ANY, \
+    (wxObjectEventFunction) (wxEventFunction) (cbBuildTreeFunction) \
+    wxStaticCastEvent( cbBuildTreeFunction, & fn ), (wxObject *) NULL ),
+
+
+
+#endif
diff --git a/src/plugins/codecompletion/classbrowser.cpp b/src/plugins/codecompletion/classbrowser.cpp
index cd96467..498113d 100644
--- a/src/plugins/codecompletion/classbrowser.cpp
+++ b/src/plugins/codecompletion/classbrowser.cpp
@@ -44,6 +44,7 @@
 #include "nativeparser.h"
 
 #include "parser/ccdebuginfo.h"
+#include <stdio.h>
 
 #define CC_CLASS_BROWSER_DEBUG_OUTPUT 0
 
@@ -76,6 +77,15 @@
     #define TRACE2(format, args...)
 #endif
 
+// below is used on wx3.x only
+//wxDEFINE_EVENT(cbBUILD_CLASSTREE_EVENT,cbBuildTreeEvent);
+
+IMPLEMENT_DYNAMIC_CLASS(cbBuildTreeEvent, wxCommandEvent)
+DEFINE_EVENT_TYPE(cbBUILD_CLASSTREE_EVENT)
+
+
+
+
 int idMenuJumpToDeclaration    = wxNewId();
 int idMenuJumpToImplementation = wxNewId();
 int idMenuRefreshTree          = wxNewId();
@@ -127,6 +137,7 @@ BEGIN_EVENT_TABLE(ClassBrowser, wxPanel)
     EVT_MENU(idCBBottomTree,                             ClassBrowser::OnCBViewMode)
 
     EVT_COMMAND(idThreadEvent, wxEVT_COMMAND_ENTER,      ClassBrowser::OnThreadEvent)
+    EVT_CBBUILD_CLASSTREE(wxID_ANY,ClassBrowser::OnBuileTree)
 END_EVENT_TABLE()
 
 // class constructor
@@ -971,3 +982,10 @@ void ClassBrowser::OnThreadEvent(wxCommandEvent& event)
             break;
     }
 }
+
+void ClassBrowser::OnBuileTree(cbBuildTreeEvent& event)
+{
+  ClassBrowserBuilderThread* thread = event.GetBuilderThreadObject();
+  if(thread)
+    thread->BuildTree();
+}
diff --git a/src/plugins/codecompletion/classbrowser.h b/src/plugins/codecompletion/classbrowser.h
index 9715c07..1a272c7 100644
--- a/src/plugins/codecompletion/classbrowser.h
+++ b/src/plugins/codecompletion/classbrowser.h
@@ -16,6 +16,7 @@
 #include "classbrowserbuilderthread.h"
 #include "parser/parser.h"
 #include "parser/token.h"
+#include "BuildTreeEvent.h"
 
 class NativeParser;
 class wxComboBox;
@@ -90,6 +91,8 @@ private:
     void OnDebugSmartSense(wxCommandEvent& event);
     void OnSetSortType(wxCommandEvent& event);
 
+    void OnBuileTree(cbBuildTreeEvent& event);
+
     void OnSearch(wxCommandEvent& event);
     bool FoundMatch(const wxString& search, wxTreeCtrl* tree, const wxTreeItemId& item);
     wxTreeItemId FindNext(const wxString& search, wxTreeCtrl* tree, const wxTreeItemId& start);
diff --git a/src/plugins/codecompletion/classbrowserbuilderthread.cpp b/src/plugins/codecompletion/classbrowserbuilderthread.cpp
index 24f05cb..5b74337 100644
--- a/src/plugins/codecompletion/classbrowserbuilderthread.cpp
+++ b/src/plugins/codecompletion/classbrowserbuilderthread.cpp
@@ -26,11 +26,12 @@
     #include <projectmanager.h>
 #endif
 
+#include "BuildTreeEvent.h"
 #include "classbrowserbuilderthread.h"
 
 // sanity check for the build tree functions, this function should only be called in a worker thread
 // also, there should be no termination requested, otherwise, it will return false
-#define CBBT_SANITY_CHECK ((!::wxIsMainThread() && m_TerminationRequested) || Manager::IsAppShuttingDown())
+#define CBBT_SANITY_CHECK (( m_TerminationRequested) || Manager::IsAppShuttingDown())
 
 #define CC_BUILDERTHREAD_DEBUG_OUTPUT 0
 
@@ -63,6 +64,8 @@
     #define TRACE2(format, args...)
 #endif
 
+#include "BuildTreeEvent.h"
+
 ClassBrowserBuilderThread::ClassBrowserBuilderThread(wxEvtHandler* evtHandler, wxSemaphore& sem) :
     wxThread(wxTHREAD_JOINABLE),
     m_Parent(evtHandler),
@@ -184,7 +187,7 @@ void ClassBrowserBuilderThread::Init(NativeParser*         np,
 }
 
 // Thread function
-
+#include <stdio.h>
 void* ClassBrowserBuilderThread::Entry()
 {
     while (!m_TerminationRequested && !Manager::IsAppShuttingDown() )
@@ -206,7 +209,10 @@ void* ClassBrowserBuilderThread::Entry()
                 ::wxMutexGuiEnter();
         }
 
-        BuildTree();
+        //BuildTree();
+        cbBuildTreeEvent evt(this,cbBUILD_CLASSTREE_EVENT);
+        if(m_Parent)
+          m_Parent->AddPendingEvent(evt);
 
         if (platform::gtk || platform::macosx)
         {
@@ -447,6 +453,8 @@ void ClassBrowserBuilderThread::BuildTree()
     // 6.) Hide&Freeze trees shown
     if (m_BrowserOptions.treeMembers)
     {
+        if(m_CCTreeCtrlBottom==NULL)
+          wxMessageBox(wxT("kcynice:classbrowserbuilderthread.cpp::BuildTree m_CCTreeCtrlBottom is null"));
         m_CCTreeCtrlBottom->Hide();
         m_CCTreeCtrlBottom->Freeze();
     }
diff --git a/src/plugins/codecompletion/classbrowserbuilderthread.h b/src/plugins/codecompletion/classbrowserbuilderthread.h
index 9369a27..3acfcb8 100644
--- a/src/plugins/codecompletion/classbrowserbuilderthread.h
+++ b/src/plugins/codecompletion/classbrowserbuilderthread.h
@@ -20,6 +20,7 @@
  */
 class ClassBrowserBuilderThread : public wxThread
 {
+    friend class ClassBrowser;
 public:
     /** the builder threads' event sent to the GUI(class browser window)*/
     enum EThreadEvent

[oBFusCATed]

ollydbg: The last patch does nothing interesting. Is it the only change that has to be made to C::B to fix the problem or other patches are required?

[ollydbg]

ollydbg: The last patch does nothing interesting. Is it the only change that has to be made to C::B to fix the problem or other patches are required?

Yes, the patch is not mature.
It is kind of proof of concept to move those operations in GUI main thread. 

[oBFusCATed]

But if I understand you correctly you're moving all operations to the UI thread, because I don't see any data stored in the event's object.

[ollydbg]

But if I understand you correctly you're moving all operations to the UI thread, because I don't see any data stored in the event's object.

The function "void ClassBrowserBuilderThread::BuildTree()" is not called in "void* ClassBrowserBuilderThread::Entry()", but in "void ClassBrowser::OnBuileTree(cbBuildTreeEvent& event)".

The custom event(cbBuildTreeEvent) just carry a "ClassBrowserBuilderThread* thread" from the worker thread to main GUI.

[oBFusCATed]

OK, but how do you know how many elements you have to add to the tree in a single event handler? As far as I see it you still need locking.

If I'm doing this I'd rather:
1. in the parser thread generate some data and store it in the symbolbrowser event object
2. post an event with the gathered data
3. continue with parsing
4. when I receive the event I'll add the data to the tree

This way it is explicitly stated how owns the data exclusively and there is no need for locking.

[ollydbg]

OK, but how do you know how many elements you have to add to the tree in a single event handler? As far as I see it you still need locking.

If I'm doing this I'd rather:
1. in the parser thread generate some data and store it in the symbolbrowser event object
2. post an event with the gathered data
3. continue with parsing
4. when I receive the event I'll add the data to the tree

This way it is explicitly stated how owns the data exclusively and there is no need for locking.

OK, I understand you idea, but we are not talking with the same lockers.
You are stating the locker for the TokenTree, but my patch try to move the build wxTreeCtrl code from worker thread to GUI thread.

Note, TokenTree locker is still there, but mostly they don't cause issue, because the wxTreeCtrl is mainly constructed after the Parser finish the job, in that time, TokenTree is always free(not owned by other one).

Back to your proposed method, to avoid the TokenTree locker when building the tree, you need to construct some data structure (some kind of a copy of a sub Tokentree?) and send to the GUI thread, but if the data structure is too large?

[MortenMacFly]

First of all the patch does not work but crashes cb at exit. Second if the data gets too large you can use a pointer. Third I don't think it's that easy.Yes, the method is called from outside but the task is still operated in the thread so it doesn't really change anything... Sorry.

[ollydbg]

First of all the patch does not work but crashes cb at exit.

Hi, Morten, thanks for the test. In-fact, If we need to build the Tree from GUI, we should totally remove the worker tree builder thread stuff.

Second if the data gets too large you can use a pointer.

Yes, just as the same way as we access to Tokentree to query suggestion list information.

Third I don't think it's that easy.Yes, the method is called from outside but the task is still operated in the thread so it doesn't really change anything... Sorry.

The tree build thread is large, also, I read some comments in the code like:

Code

// 9.) Expand item --> Bottleneck: Takes ~4 secs on C::B workspace
....
// 11.) Select the item saved before --> Bottleneck: Takes ~4 secs on C::B workspace

I won't let the GUI to hang for 8 seconds, so I won't step further in this direction for now. 

[kipade]

Hi All, thanks for all attention about this topic. I wanna say, the simple code was just a test for that, just to see if this can fixed the crash bug, a joke.
Yes, if need finally perfect fix for that issue, some more slight design for that might needed. Of course we can drive a tree control from wxTreeCtrl, it seems more familar with OO rule. But, there still have some other choices such as my moved BuidTree. I think, if all the operations could be separated with data, and GUI, one class only do which it should, all the things become easy. I think.
If it still no official support, I might continue to complete it by my hand, because I need it work under linux. Any idea is welcome.

[ollydbg]

If it still no official support, I might continue to complete it by my hand, because I need it work under linux. Any idea is welcome.

Do you think it is definitely the codecompletion's browser tree which cause the crash issue?
I haven't seen that on our forum about such issue, can you show us the crash backtrace?

[oBFusCATed]

Are we sure what is taking all the time when building the tree from the main thread?
Has any one run a profiler to see what is slowing things down? Most of the times it is something quite unexpected, because people are really bad at guessing bottleneck!

If someone is willing to bring back all UI building code to the main thread and provide a patch, I'll run some profiling tools on it and I'll try to improve it.

[MortenMacFly]

I'm afraid there is nothing to bring back as imho it has always been like that...

[oBFusCATed]

I think Loaden has moved it in a thread, but anyway, lets say to move the code to the UI thread in order to make it easier to profile.

[ollydbg]

I think Loaden has moved it in a thread, but anyway, ...

No, the wxTreeCtrl builder thread has a long history, it exist before Loaden became a C::B developer.

[kipade]

If it still no official support, I might continue to complete it by my hand, because I need it work under linux. Any idea is welcome.

Do you think it is definitely the codecompletion's browser tree which cause the crash issue?
I haven't seen that on our forum about such issue, can you show us the crash backtrace?

Im sure. I will upload a crash trace when I have some free time to have a try.

[ollydbg]

OK, four years later, and I still think I need symbol tree. So, this is the patch against the latest svn head.
I build it under 32bit gcc 7.2 under Windows with wx 3.1.1.

I think we can "disable" the option to show the whole project's tokens in the tree, because this should take too long. But for tokens in a single source file, it should be OK. I don't see time lags.

[oBFusCATed]

Can you explain how this patch works and what it tries to achieve?
I've tried to look at it, but I'm not familiar with the code, so I cannot really say if it is a good idea.

Generally my understanding is that without a major redesign, the Class Browser would be a constant cause of problems. As far as I can see this is not a major redesign, but a try to workaround the problem...

[ollydbg]

Can you explain how this patch works and what it tries to achieve?
I've tried to look at it, but I'm not familiar with the code, so I cannot really say if it is a good idea.

The patch is quite simple, it just translate all the "tree building" code snippets from running in a worker thread to the main GUI thread. Those code snippets was running both from the main GUI thread and worker thread. By using this patch, a custom event is used, when we need to build the tree or operating on the tree, the event if fired, and the final operations all happens in the main GUI thread.

Generally my understanding is that without a major redesign, the Class Browser would be a constant cause of problems. As far as I can see this is not a major redesign, but a try to workaround the problem...

Yes, I agree, this patch is just a workaround.

[oBFusCATed]

Also I don't think it is a good idea to push this now. It will be good if you can wait a bit.

[ollydbg]

Hi, @kipade, I see one issue about your patch (cc-build-symbol-tree-in-main-thread.patch), I see that the class of the current cpp/h file only shown when I collapse the root node and expand it again, see gif screen shot here: https://github.com/asmwarrior/cbdiscuss/blob/master/symbol_tree1.gif