Redundant Crash in CCManager::OnShowCallTip()

[darksquall57]

Hi,

I'm working with codeblocks for a long time now, and sometimes I have some crashes and if I restart codeblocks it's ok.
But this time it's not the case, I have a crash which occurs after just a few actions ( moving in the code, writing a function call, hitting ";" to finish my line, ... ).

I cannot share my code, but here is the call I retrieved from the Ubuntu crash report.

Code

  <stack>
    <frame level="0"/>
    <frame level="1" function="wxStringBase::operator=(wxStringBase const&amp;)" offset="00000012"/>
    <frame level="2" function="CCManager::OnShowCallTip(CodeBlocksEvent&amp;)" offset="0000017a"/>
    <frame level="3" function="Manager::ProcessEvent(CodeBlocksEvent&amp;)" offset="00000080"/>
    <frame level="4" function="CCManager::OnDeferredCallTipShow(wxCommandEvent&amp;)" offset="000000a3"/>
    <frame level="5" function="wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&amp;, wxEvtHandler*, wxEvent&amp;)" offset="00000056"/>
    <frame level="6" function="wxEvtHandler::SearchDynamicEventTable(wxEvent&amp;)" offset="0000004f"/>
    <frame level="7" function="wxEvtHandler::ProcessEvent(wxEvent&amp;)" offset="00000092"/>
    <frame level="8" function="wxEvtHandler::ProcessPendingEvents()" offset="00000068"/>
    <frame level="9" function="wxAppConsole::ProcessPendingEvents()" offset="00000051"/>
    <frame level="10" function="wxAppBase::ProcessIdle()" offset="0000001e"/>
    <frame level="11"/>
    <frame level="12" function="g_main_context_dispatch" offset="00000135"/>
    <frame level="13"/>
    <frame level="14" function="g_main_loop_run" offset="0000006a"/>
    <frame level="15" function="gtk_main" offset="000000a7"/>
    <frame level="16" function="wxEventLoop::Run()" offset="0000003a"/>
    <frame level="17" function="wxAppBase::MainLoop()" offset="0000004c"/>
  </stack>

I hope this will help you to find it

[ollydbg]

Thanks fore the report.

Indeed, we have discussion about this issue before. That's the reason why CCManager::OnDeferredCallTipShow() was introduced, but it looks like we haven't really fix the real problem yet.

One possible reason is that wxString(in wx2.8.12) is not thread safe.

[oBFusCATed]

Can you please install the debug packages (-dbg).
Start codeblocks under gdb (gdb /usr/bin/codeblocks) and when it crashes please run the "thread apply all bt" in the gdb command prompt.

If you can reproduce this in a simple project or some big opensource project it will be invaluable.

[ollydbg]

@darksquall57, can you try to build CB against wx3.x, which has better thread safe wxString implementation. Thanks.

[darksquall57]

Can you please install the debug packages (-dbg).

you mean calling "./configure --enable-debug" ?

If you can reproduce this in a simple project or some big opensource project it will be invaluable.

Sadly, I can't reproduce it for now, but I'm working on it.

@darksquall57, can you try to build CB against wx3.x, which has better thread safe wxString implementation. Thanks.

I'll try that too, thanks.

[oBFusCATed]

you mean calling "./configure --enable-debug" ?

If you're building cb yourself then yes.
If you're using binary packages then there is a package that has debug info in it. You have to just install it.

[ollydbg]

About the crash issue: I firstly guess this is the thread issue, but when I check the code, I see that in ccmanager

Code

void CCManager::OnShowCallTip(CodeBlocksEvent& event)
{
    event.Skip();

    int tooltipMode = Manager::Get()->GetConfigManager(wxT("ccmanager"))->ReadInt(wxT("/tooltip_mode"), 1);
    // 0 - disable
    // 1 - enable
    // 2 - force single page
    // 3 - keybound only
    if (tooltipMode == 0)
        return;

    cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
    if (!ed)
        return;

    cbCodeCompletionPlugin* ccPlugin = GetProviderFor(ed);
    if (!ccPlugin)
        return;

    cbStyledTextCtrl* stc = ed->GetControl();
    if (!stc)
        return;

    int pos = stc->GetCurrentPos();
    int argsPos = wxSCI_INVALID_POSITION;
    wxString curTip;
    if (!m_CallTips.empty())
        curTip = m_CurCallTip->tip;
    m_CallTips = ccPlugin->GetCallTips(pos, stc->GetStyleAt(pos), ed, argsPos);
    ...

When call the ccPlugin's GetCallTips() function, I see that there is a check:

Code

std::vector<CodeCompletion::CCCallTip> CodeCompletion::GetCallTips(int pos, int style, cbEditor* ed, int& argsPos)
{
    std::vector<CCCallTip> tips;
    if (!IsAttached() || !m_InitDone || style == wxSCI_C_WXSMITH || !m_NativeParser.GetParser().Done())
        return tips;

    int typedCommas = 0;
    wxArrayString items;
    ...

You see, there is a check "!m_NativeParser.GetParser().Done()", This means there is no other thread running on the TokenTree(At least in the active Parser object), so my question is: do you open several cbp projects in C::B? This is the chance that other Parser objects are running parsing jobs in its thread pool.

Another guess: as Morten has once said the crash line is here

Code

curTip = m_CurCallTip->tip;

My guess is that if m_CurCallTip(an iterator) could point to wrong elements of the m_CallTips(vector)?
EDIT: Maybe, you have two ccPlugins, and with one plugin, you have update the iterator, but this iterator point to the vector updated by another ccPlugin. (This is just a guess)

Since I don't have a chance to reproduce this crash issue, I have all my guesses by reading the CC and CCmanager's source code.



 

[ollydbg]

@Alpha, can you explain what the meaning of  "// search long term recall" and "// search short term recall" in the function void CCManager::OnShowCallTip(CodeBlocksEvent& event)?

I can't quite understand the related code snippet. Since you have some directory, and hash code in those code.

My guess is that you want to enable a cache?  For example:

Code

void f(int a, float b, double c);  
void f(char d, void *h);

When user select a call tip of function "void f(char d, void *h);" in the call tip window, the next time when the call tip window is shown, the "void f(char d, void *h);"  is automatically selected?

Thanks.

[ollydbg]

I found that one issue is that sometimes, the tip window doesn't cancel correctly.

Code

void cbStyledTextCtrl::CallTipCancel()
{
    if (!m_tabSmartJump)
        wxScintilla::CallTipCancel();
}

This is called inside the DoShowTips() function.

Code

void CCManager::DoShowTips(const wxStringVec& tips, cbStyledTextCtrl* stc, int pos, int argsPos, int hlStart, int hlEnd)
{
    ...
    if (stc->CallTipActive() && m_LastTipPos != pos)
        stc->CallTipCancel(); // force tip popup to invalidate (sometimes fails to otherwise do so on Windows)
    stc->CallTipShow(pos, tip);
    if (hlStart >= 0 && hlEnd > hlStart)
        stc->CallTipSetHighlight(hlStart, hlEnd);
    m_LastTipPos = pos;
}

stc->CallTipCancel();  won't actually cancel the calltip, because the m_tabSmartJump==true.
Later, we call stc->CallTipShow() again.

EDIT: I see that in this case, another calltip is allocated from the wxScintilla..... So, how about the first calltip?

[ollydbg]

Oh, I see some code snippet:

Code

// cbEVT_EDITOR_TOOLTIP
void CCManager::OnEditorTooltip(CodeBlocksEvent& event)
{
    ...
    if (!ccPlugin || pos < 0 || pos >= stc->GetLength())
    {
        if (stc->CallTipActive() && event.GetExtraLong() == 0 && m_CallTipActive == wxSCI_INVALID_POSITION)
            static_cast<wxScintilla*>(stc)->CallTipCancel();
        return;
    }

Here, why you use the static_cast<wxScintilla*>(stc)->CallTipCancel();? You want to skip the condition check on m_tabSmartJump? and force to cancel it?

[Alpha]

Here, why you use the static_cast<wxScintilla*>(stc)->CallTipCancel();? You want to skip the condition check on m_tabSmartJump? and force to cancel it?

Yes; calling base class to force it to cancel.

@Alpha, can you explain what the meaning of  "// search long term recall" and "// search short term recall" in the function void CCManager::OnShowCallTip(CodeBlocksEvent& event)?

I can't quite understand the related code snippet. Since you have some directory, and hash code in those code.

They are remember which page on the calltip was last selected, and show that again first next time it is requested.
Short term recall caches the current calltip, so it can be displayed exactly if a calltip is re-requested in the same location.
Long term recall uses two dictionaries, the first one based on the tip's content, and will display the last shown page when a new calltip is requested that has the same content.  The second (fuzzy) dictionary remembers which page was shown based on the typed word prefix (this is to support FortranProject, which uses more dynamic calltips).

[ollydbg]

Here, why you use the static_cast<wxScintilla*>(stc)->CallTipCancel();? You want to skip the condition check on m_tabSmartJump? and force to cancel it?

Yes; calling base class to force it to cancel.

OK, thanks, then do we use the wxScintilla::CanTipCancel() in the void CCManager::OnEditorTooltip()?

@Alpha, can you explain what the meaning of  "// search long term recall" and "// search short term recall" in the function void CCManager::OnShowCallTip(CodeBlocksEvent& event)?

I can't quite understand the related code snippet. Since you have some directory, and hash code in those code.

They are remember which page on the calltip was last selected, and show that again first next time it is requested.
Short term recall caches the current calltip, so it can be displayed exactly if a calltip is re-requested in the same location.
Long term recall uses two dictionaries, the first one based on the tip's content, and will display the last shown page when a new calltip is requested that has the same content.  The second (fuzzy) dictionary remembers which page was shown based on the typed word prefix (this is to support FortranProject, which uses more dynamic calltips).

Thanks very much. I briefly understand this.
I'm currently add those text to the comment part in ccmanager.h/cpp files 
Feel free to adjust.

[darksquall57]

Wow ! A lot of work and discussion since my last post

You see, there is a check "!m_NativeParser.GetParser().Done()", This means there is no other thread running on the TokenTree(At least in the active Parser object), so my question is: do you open several cbp projects in C::B?

I'm working only with 1 project opened at the same time.

EDIT: Maybe, you have two ccPlugins, and with one plugin, you have update the iterator, but this iterator point to the vector updated by another ccPlugin. (This is just a guess)

I don't know, how can I figure this out ?

[ollydbg]

You see, there is a check "!m_NativeParser.GetParser().Done()", This means there is no other thread running on the TokenTree(At least in the active Parser object), so my question is: do you open several cbp projects in C::B?

I'm working only with 1 project opened at the same time.

OK, so even a single project could cause the crash bug.

EDIT: Maybe, you have two ccPlugins, and with one plugin, you have update the iterator, but this iterator point to the vector updated by another ccPlugin. (This is just a guess)

I don't know, how can I figure this out ?

The Fortran code completion plugin is a SVN external module of our C::B's SVN repo, you can see its source in "\src\plugins\contrib\FortranProject" if you use SVN. If you have already build this plugin, you can disable it from the Plugin manager in the Menu->Plugins->Manager plugins.

[ollydbg]

I think I may find the reason why the crash happens by reading the source code. 

We have such code:

Code

void CCManager::OnShowCallTip(CodeBlocksEvent& event)
{
    event.Skip();

    int tooltipMode = Manager::Get()->GetConfigManager(wxT("ccmanager"))->ReadInt(wxT("/tooltip_mode"), 1);
    // 0 - disable
    // 1 - enable
    // 2 - force single page
    // 3 - keybound only
    if (tooltipMode == 0)
        return;

    cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
    if (!ed)
        return;

    cbCodeCompletionPlugin* ccPlugin = GetProviderFor(ed);
    if (!ccPlugin)
        return;

    cbStyledTextCtrl* stc = ed->GetControl();
    if (!stc)
        return;

    int pos = stc->GetCurrentPos();
    int argsPos = wxSCI_INVALID_POSITION;
    // save the current tip shown text for later recalling
    wxString curTip;
    if (!m_CallTips.empty())
        curTip = m_CurCallTip->tip;
    m_CallTips = ccPlugin->GetCallTips(pos, stc->GetStyleAt(pos), ed, argsPos);
    // since m_CallTips get updated, we should update the m_CurCallTip
    // But what about if m_CallTips is empty?

    if (!m_CallTips.empty() && (event.GetInt() != FROM_TIMER || argsPos == m_CallTipActive))
    {
        ...
        Here, we update the m_CurCallTip
        ...
    }

Please see my comments

Code

    // since m_CallTips get updated, we should update the m_CurCallTip
    // But what about if m_CallTips is empty?

I mean, if m_CallTips is modified, we should update the m_CurCallTip, since m_CurCallTip is an iterator points to the m_CallTips. If m_CallTips is empty, we have no chance to update the m_CurCallTip, then m_CurCallTip becomes a wild pointer.
Am I right? I haven't tested and debug the code, I just read the cc related code time and time again.

[oBFusCATed]

If m_CallTips is empty, we have no chance to update the m_CurCallTip, then m_CurCallTip becomes a wild pointer.

Probably the iterator should be reset by default constructing it before updating the m_CallTips.
The code would look something like myit=MyIterator().
Please check the documentation of std::vector or whatever the type of m_CallTips is.

[ollydbg]

If m_CallTips is empty, we have no chance to update the m_CurCallTip, then m_CurCallTip becomes a wild pointer.

Probably the iterator should be reset by default constructing it before updating the m_CallTips.
The code would look something like myit=MyIterator().
Please check the documentation of std::vector or whatever the type of m_CallTips is.

It looks like there is no way to check whether an iterator is valid, see: c++ - Checking if an iterator is valid - Stack Overflow.

I don't have a good idea to fix our CC issue.

[ollydbg]

One idea to fix this issue:

Code

    wxString curTip;
    // HERE, Check whether m_CurCallTip is invalid(point to the end of m_CallTips)
    if (!m_CallTips.empty() && m_CurCallTip != m_CallTips.end())
        curTip = m_CurCallTip->tip;
    m_CallTips = ccPlugin->GetCallTips(pos, stc->GetStyleAt(pos), ed, argsPos);
    // since m_CallTips get updated, we should update the m_CurCallTip, they are done in
    // the following if/else statement.

    if (!m_CallTips.empty() && (event.GetInt() != FROM_TIMER || argsPos == m_CallTipActive))
    {
        ....
        m_CurCallTip = m_CallTips.begin();
        ....
        // HERE, m_CurCallTip is always valid(points to element of  m_CallTips)
        DoUpdateCallTip(ed);
    }
    else if (m_CallTipActive != wxSCI_INVALID_POSITION)
    {
        static_cast<wxScintilla*>(stc)->CallTipCancel();
        m_CallTipActive = wxSCI_INVALID_POSITION;
        // HERE, make m_CurCallTip invalid
        m_CurCallTip = m_CallTips.end();
    }

See the comments after "// HERE".

EDIT: since only the if clause will call the "DoUpdateCallTip(ed);", the else clause just cancel the calltip.

[ollydbg]

This is the testing patch:

Code

 src/sdk/ccmanager.cpp | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/sdk/ccmanager.cpp b/src/sdk/ccmanager.cpp
index a02c9e6..c3d244c 100644
--- a/src/sdk/ccmanager.cpp
+++ b/src/sdk/ccmanager.cpp
@@ -912,10 +912,15 @@ void CCManager::OnShowCallTip(CodeBlocksEvent& event)
 
     int pos = stc->GetCurrentPos();
     int argsPos = wxSCI_INVALID_POSITION;
+    // save the current tip shown text for later recalling
     wxString curTip;
-    if (!m_CallTips.empty())
+    // check whether m_CurCallTip is invalid(point to the end of m_CallTips)
+    if (!m_CallTips.empty() && m_CurCallTip != m_CallTips.end())
         curTip = m_CurCallTip->tip;
+
     m_CallTips = ccPlugin->GetCallTips(pos, stc->GetStyleAt(pos), ed, argsPos);
+    // since m_CallTips get updated, we should update the m_CurCallTip, they are done in
+    // the following if/else statement.
     if (!m_CallTips.empty() && (event.GetInt() != FROM_TIMER || argsPos == m_CallTipActive))
     {
         int lnStart = stc->PositionFromLine(stc->LineFromPosition(pos));
@@ -983,6 +988,8 @@ void CCManager::OnShowCallTip(CodeBlocksEvent& event)
     {
         static_cast<wxScintilla*>(stc)->CallTipCancel();
         m_CallTipActive = wxSCI_INVALID_POSITION;
+        // make m_CurCallTip invalid
+        m_CurCallTip = m_CallTips.end();
     }
 }
 

With the log message:

Code

* SDK: ccmanager: when m_CallTips get updated, also update the iterator.

This try to fix a crash reported here:
http://forums.codeblocks.org/index.php/topic,20489.0.html
Because if we don't update the iterator(m_CurCallTip), the iterator becomes
a wild pointer.

It is against SVN 10391.
Comments are welcome.

[oBFusCATed]

After looking at the docs for vector/iterator I think that the iterator is not a good choice for this case.
If I were you, I'd replace it with a simple index variable.

[ollydbg]

After looking at the docs for vector/iterator I think that the iterator is not a good choice for this case.
If I were you, I'd replace it with a simple index variable.

Indeed, this idea also comes to my mind, but that need a lot of code changes, I'm not fully understand the ccmanager.h/cpp, so let's do it step by step. Also, I need Alpha's comment.
Also, I want to know whether it fixes the crash issue. Since I don't have this crash in my computer even without this patch.

[ollydbg]

Here, why you use the static_cast<wxScintilla*>(stc)->CallTipCancel();? You want to skip the condition check on m_tabSmartJump? and force to cancel it?

Yes; calling base class to force it to cancel.

OK, thanks, then do we use the wxScintilla::CanTipCancel() in the void CCManager::OnEditorTooltip()?

I think I can answer this question myself, this won't cause some memory leak if the tip window is not cancled, then we try to show it again, the already opened tip window will be reused. When we call stc->CallTipShow(pos, tip);, we will finally reach the blow code:

Code

void ScintillaWX::CreateCallTipWindow(PRectangle) {
    if (! ct.wCallTip.Created() ) {
        ct.wCallTip = new wxSCICallTip(sci, &ct, this);
        ct.wDraw = ct.wCallTip;
    }
}

[oBFusCATed]

Indeed, this idea also comes to my mind, but that need a lot of code changes, I'm not fully understand the ccmanager.h/cpp, so let's do it step by step.

I think the changes are quite trivial and low-risk.