#1444 Legacy Codecompletion SymbolBrowser misbehavior and crashes

[pecan]

See: Report by tigerbeard
https://forums.codeblocks.org/index.php?topic=25652.msg174568#msg174568

Summary:
1) Re: rev 13403
2) Using settings: one parser per project
3) Symbol browser fully closes when updated
4) The last tree positon & open stat is lost
5) Tree may crash CB when clicking item in tree
6) Using one parser for the workspace, tree closes on double click on class in non-editor (open) file

[pecan]

Suggested re-creation methods:
https://forums.codeblocks.org/index.php?topic=25652.msg174595#msg174595
https://forums.codeblocks.org/index.php?topic=25652.msg174594#msg174594

I have a test using the workspace trunk/src/CodeBlocks_wx30-unix.workspace

First Test
Set Settings/Editor/CC/CCParser/UseOneParserForWholeWorkspace
Set Symbol Browser to "Current Projects Symbols"
Make sure no files are open in the editor
Activate Project wxSmith/ContribItems
Switch to symbol Browser
Open a class in the tree, e.g. wxsAxis and double clikc on the constructor
Obvservation: The file is opened, and the tree is refeshed and closed
Expected: The file is opened, and the tree remains open and wxAxis ctor is selected.

Second Test
Set CC option to OneParserPerProject. It will ask to reparse. Set OK
Repeat Test1. it should show the expected behaviour
Enter Project Tab and select project Clangd client
Switch to Symbol Browser Tab.
Set Symbol Browser to "Current Projects Symbols".
Observe Popup : "This feature is not supported in combination with the option "one parser per whole workspace".
If you got the popup there are several ways to get to the correct display. Beside restarting CB switching to FileSymbold and selecting a file in the new project and switching back worked.
Open naemspace Doxygen
double click on DoxygenParser class -> header opens. tree is stable
double click on DoxygenParser ctor -> cpp opens. tree is stable
Add a space in the open doxygen_parser.h file and press CTRL-S to save
Obvservation: The tree is refeshed and closed
Expected: The tree is refeshed and remains open

[pecan]

Result for first test (pecan)
Tree is refreshed and the tree stays OPEN. but the selection is lost.
But if "Display bottom tree" is checked (an option of the tree root right-click),
the results are as expected. The tree does not refresh when parsing the next project and the selection remains selected.

[Miguel Gimenez]

I can not check right now, but deleting the last condition in classbrowser.cpp:1150 may work:

    if (parent.IsOk() && m_ClassBrowserBuilderThread && m_Parser && m_Parser->ClassBrowserOptions().treeMembers)
    {
        m_CCTreeCtrl->SelectItem(parent);
        m_CCTreeCtrl->EnsureVisible(parent);
    }

The last condition is a check for the bottom tree presence.

[pecan]

Re: first test.
The tree selection is indeed lost on linux when refreshing the tree.
I could not re-recreate a closed tree situation unless there was no file of the activated project open. Legacy CC is driven from "OnEditorActivated()" requiring that at least one file of the project be activated.

Re: second test.
I was unable to re-create the situation as described . It also loses the tree selection, but I never experienced a closed tree unless there was no open file of the project.

Note 1: Legacy CodeCompletion is driven from "OnEditorActivated()". It is dependent on at least one file of the project being open in an editor.
Confusing situations arise in that, when a workspace is first loaded, the Symbols tree is filled with symbols from the active project of the workspace without an open editor. However, when activating a subsequent project, the Symbols tree will not be filled until a file of the project is also activated.

Note 2: When activating a different project of a workspace, the symbols tree can be slow to update to the newly activated project; especially while it's parsing the project. Even when selecting to show "Current project's symbols" or "Refesh tree" (from the Symbols context men) . Even more confusing is that the tree will continue to display the de-activated project's symbols or closes the tree (if no file of the project is open).

My environment: Linux Mint 21.x, wxWidgets 3.0, Head rev 13426 .
Windows 11, wxWidgets 3.2.4, Head rev 13422.

[pecan]

I think I might be able to fix the "lost selection" situation, but I really do not want to tackle the situation when there is no open file of the project and the tree gets updated to a "closed" state.
Attempting to "fix" it will just cause more annoyances since it looks to me that the whole Symbols tree display is built on, and driven from , "OnEditorActivated()".
Changing that now would introduce another can of worms.

I recognize that this is a maddening annoyance and will work to make sure it does not occur in Clangd_client plugin.

[Tiger Beard]

but I really do not want to tackle the situation when there is no open file of the project and the tree gets updated to a "closed" state.

I tested that and it seems to work quite conistently. When switching projects the project tab makes it very easy to pick any file from the project and then switching to the Symbols tab will load the new tree.
I think as procedure that is a very good workaround, but the point it how to know that this is the procedure. Maybe some Initial "don't show any more" Dialog when using the Symbols/View wxChoice ?

Note: Interesting I find here that when I use that wxChoice box to switch from CurrentProject Symbols to Whole workspace I get the error message "not possible with one parser for workspace" but the settings is set to "one parser per project". Maybe thats a minor update bug.

[Tiger Beard]

The problem shows on my system
Linux Ubuntu/xfce (18.04 and 22.04 shows the same behaviour).
wxWidgets is: 3.0.5

[Tiger Beard]

I tried to debug into a case from Test2 where the tree is closed on saving a header.

What I see is the following sequence

 //entry is here and almost always the build tree is called
 ClassBrowser::UpdateClassBrowserView(..)    
     ThreadedBuildTree(activeProject);       

 //But while this builds the tread the first time, it initialized t every ime
  ClassBrowser::ThreadedBuildTree(..)
     m_ClassBrowserBuilderThread->Init(...)

 //But there the member var m_CCTreeTop is always recreated:
 ClassBrowserBuilderThread::Init(...)
   ..
   if( m_CTreeTop)    { delete m_CCTreeTop; }


  // That means that in BuildTree the tree is empty and it has no expanded items to save. 
  ClassBrowserBuilderThread::BuildTree()
     ..
      SaveExpandedItems( m_CCTreeTop, root, 0); 

My tracing confirms that the m_CCTreeTop had no root and the recursive for loop in SaveExpandedItems was not even entered. The m_ExpandedVect was empty.

At the CRC32 comparions the trees were considered different so it goes to replacement mode. Would't at that point he code should have to returne since no actual change as been done ( by adding a space to the header file)?

However, it should still work, because in my use case the class was selected in the tree and the two calls below were executed and should have reopened the tree to the selection (shouldn't they?)

 CallAfter( &ClassBrwoser::SaveSelectedItem);
  ..
 CallAfter( &ClassBrwoser::SelectSavedItem);

SaveSelectedItem functions filled and read the variable m_SelectedPath correctly. In SelectSavedItem() it parses the m_CCTreeTop and fails because the tree only has top items, so the function creates a item.IsOk()==false

item = m_CCTreeCtrl->GetFirstChild( imtem, cookie2);  // line 1134

Not sure which bits are by design.

Is is normal to get from gdb debug stepping a performance of 10seconds per single step in the CB code? I use a decent quad core CPU with 8GB RAM.

[pecan]

There was recent change to building the Symbols tree because it was thought that the tree was being leaked.

https://sourceforge.net/p/codeblocks/code/13392/tree//trunk/src/plugins/codecompletion/classbrowserbuilderthread.cpp?diff=51421f1dc431436b6eb31d56:13391

The above patch was applied sometime back because it was leaking the previous Symbols tree. Revert it in your code to see if this is what is causing the problem. If so, we'll have to find a different solution to the leak.

[Tiger Beard]

Thats would explain why I did not see that behaviour in an older version.

Revert it in your code to see if this is what is causing the problem

willdo

PS: I have changed my post above after further testing.

[Tiger Beard]

Yes. After the change TestCase 2 happend only the first time which is expected since its the same situation as with the patch.
Further changes to the header do not cause the tree to change.

What I did was to comment out this patch

// m_topCrc32         = CRC32_CCITT;
//    m_bottomCrc32      = CRC32_CCITT;
//    if (m_CCTreeTop)
//    {
//        delete m_CCTreeTop;
//    }
//    if (m_CCTreeBottom)
//    {
//        delete m_CCTreeBottom;
//    }

and replaced it by this

+    if ( !m_CCTreeTop )
+    {
+        m_CCTreeTop = new CCTree();
+        m_topCrc32  = CRC32_CCITT;
+    }
+    if ( !m_CCTreeBottom )
+    {
+        m_CCTreeBottom = new CCTree();
+        m_bottomCrc32  = CRC32_CCITT;
+    }

[pecan]

Is is normal to get from gdb debug stepping a performance of 10seconds per single step in the CB code? I use a decent quad core CPU with 8GB RAM.

No, except for the first request to gdb. My stepping is almost instant, but I'm usually using windows gdb 10. But even on linux, it just a little bit slower.
I run linux Mint 21.x under VMware.
pecan@LinuxMint:~/proj/cbTest/trunk/src/output30$ gdb --version
GNU gdb (Ubuntu 12.1-0ubuntu1~22.04) 12.1
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

I just ran CodeCompletion and trap on OnEditorActivated, then stepped through it. It was almost instant.

BTW: That patch number should be 13392 not 1339.

[Tiger Beard]

I just ran CodeCompletion and trap on OnEditorActivated, then stepped through it. It was almost instant.

Thanks that was very helpful, now I know for sure that there is an issue.
I use the same gdb version. First step is always awfully long and also on first break point call stack is missing which is very bad for crash debugging. Also I have this serious issue with breakpoints that I can not delete (they are still active). So I guess there somethig seriously wrong on my system. Trouble is I have this on multiple computers running 22.04 and 18.04.

[pecan]

I remember these problems from awhile back.
The slow first reply from gdb is still a problem and (sometimes) I cannot remove a breakpoint.
But I've never had a problem with missing stack.
Be sure the call stack is enabled from withing the debugger, not the debuggee CB window.

I also found that I had to save the perspective to keep showing the stack.

Also, be sure to run with the "codeblocks.exe -d" to catch all the codecompletion messages.

[pecan]

@ Tiger Beard
Your patch appears to work on Window ok.
But we still have to activate an editor window for the Symbols tree to be updated to a newly activated project. That's expected, because the original developer designed it that way.

I was using "on parser per project". I'll now try "one parser per workspace".
It also works for "one parser per workspace". Again, a file of the project must be activated to update the Symbols tree.

[Tiger Beard]

Yes I can confirm that with "one per workspace" I also can switch between projects using he first-open-file procedure. Great.

With the "one-per-workspace" I am getting a works the tree closing behaviour. It seems to be closing for each namespace for the first time.
I restarted with "one per project" and in only closes on the very first selection.

However, from what limited understanding I have now it seems that the tree is still only created to the root level on the very first iteration. I have not explanation about the differences due to he parser options, but I guess the fix did not fully resolve root cause.
But at least my system is fully workable with the "per-project" now, so its a major improvement.

[pecan]

Quote Tiger Beard: " But at least my system is fully workable with the "per-project" now, so its a major improvement."

Great! I'll apply your fix to head.

I removed "One parser per workspace" from Clangd_client because I was having such problems with it. When changing the parser modes from one to the other (one per project vs workspace) caused such convoluted coding I got confused as to what was going on. That's a sure sign to me to avoid the complexity.

[pecan]

[Tiger Beard]

Terriffic, thanks a lot for your help!

[Tiger Beard]

@pecan

I think the current trunk is missing one required change for this to work. Without the following function the Test2 misbehaviour can still be observed.

Whem the top tree has SavedItems this function parses them, but when the BottomTree is not activated the SavedItems are not applied.

File: codecompletion/classbrowser.cpp
old code

void ClassBrowser::SelectSavedItem()
{
  ... 
    if (parent.IsOk() && m_ClassBrowserBuilderThread && m_Parser && m_Parser->ClassBrowserOptions().treeMembers)
    {
        m_CCTreeCtrl->SelectItem(parent);
        m_CCTreeCtrl->EnsureVisible(parent);
    }

New code (treeMembers removed)

void ClassBrowser::SelectSavedItem()
{
  ... 
    if (parent.IsOk() && m_ClassBrowserBuilderThread && m_Parser )
    {
        m_CCTreeCtrl->SelectItem(parent);
        m_CCTreeCtrl->EnsureVisible(parent);
    }

[pecan]

@ Tiber beard

Would you move this last change to new ticket since this one is already closed?
Also would you make the change to your system and run it for a while to see if there will be any side effects.

I don't usually run Linux, so it'd be nice to test your change for awhile in your enviornment.

Thanks