Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

Several improvements to Code Completion plugin

<< < (17/29) > >>

Huki:
A bugfix for crash when trying to cancel the ongoing project parsing (to reparse, quit CB, etc). Easiest way to reproduce is to select "Project -> Reparse current project" twice successively (or once before the initial parsing has finished). It's in fact a mutex deadlock, so here is the patch:

--- Code: ---Index: src/plugins/codecompletion/parser/parser.cpp
===================================================================
--- src/plugins/codecompletion/parser/parser.cpp (revision 9271)
+++ src/plugins/codecompletion/parser/parser.cpp (working copy)
@@ -385,11 +385,14 @@
 
 Parser::~Parser()
 {
-    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)
+    //FIX: There was a deadlock in TerminateAllThreads() when calling DeleteParser() before
+    // parsing has finished. Moved s_ParserMutex lock below and updated TerminateAllThreads().
 
     DisconnectEvents();
     TerminateAllThreads();
 
+    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)
+
     if (ParserCommon::s_CurrentParser == this)
         ParserCommon::s_CurrentParser = nullptr;
 
@@ -851,6 +867,8 @@
 
 void Parser::TerminateAllThreads()
 {
+    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)
+
     while (!m_PoolTask.empty())
     {
         PTVector& v = m_PoolTask.front();
@@ -859,6 +877,11 @@
         m_PoolTask.pop();
     }
 
+    CC_LOCKER_TRACK_P_MTX_UNLOCK(ParserCommon::s_ParserMutex)
+
+    //NOTE: This should not be locked with s_ParserMutex, otherwise we'll be stuck in an
+    // infinite loop below since the worker thread also enters s_ParserMutex.
+    // In fact cbThreadPool maintains it's own mutex, so m_Pool is probably threadsafe.
     m_Pool.AbortAllTasks();
     while (!m_Pool.Done())
         wxMilliSleep(1);

--- End code ---

And another patch to speed up canceling the ongoing parser:

--- Code: ---Index: src/plugins/codecompletion/nativeparser.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser.cpp (revision 9271)
+++ src/plugins/codecompletion/nativeparser.cpp (working copy)
@@ -616,14 +616,20 @@
 
     if (m_ParsedProjects.empty())
     {
-        if (it->second == m_Parser)
-          SetParser(m_TempParser); // Also updates class browser
-
         wxString log(F(_("NativeParser::DeleteParser(): Deleting parser for project '%s'!"), prj.wx_str()));
         CCLogger::Get()->Log(log);
         CCLogger::Get()->DebugLog(log);
 
         delete it->second;
+
+        //NOTE: Moved here as RemoveLastFunctionChildren() in SetParser() takes quite a while if
+        // parsing hasn't finished yet. Just delete the entire parser (above), then change to m_TempParser.
+        if (it->second == m_Parser)
+        {
+            m_Parser = nullptr;
+            SetParser(m_TempParser); // Also updates class browser
+        }
+
         m_ParserList.erase(it);
 
         return true;
@@ -1253,7 +1275,9 @@
     if (m_Parser == parser)
         return;
 
-    RemoveLastFunctionChildren(m_Parser->GetTokenTree(), m_LastFuncTokenIdx);
+    if (m_Parser)   //ADD, in case parser deleted before calling SetParser().
+        RemoveLastFunctionChildren(m_Parser->GetTokenTree(), m_LastFuncTokenIdx);
+
     InitCCSearchVariables();
     m_Parser = parser;
 

--- End code ---

ollydbg:
Hi, Huki, thanks, would you mind to update your local copy of C::B to the latest SVN? I have some tiny issue to apply your patch, though I can manually applied it, but it looks like some of the code I have already changed.

In the latest revision (rev9807), I have already add a comments here in the function.

--- Code: ---void Parser::TerminateAllThreads()
{
    // FIXME (ollydbg#1#): Do we need to use a mutex to protect the m_PoolTask accessing?
    while (!m_PoolTask.empty())
    {
        PTVector& v = m_PoolTask.front();
        for (PTVector::iterator it = v.begin(); it != v.end(); ++it)
            delete *it;
        m_PoolTask.pop();
    }

--- End code ---
Look at the FIXME.

BTW, in one testing branch of my local copy, I have try to remove all the lockers in CC (locker of the Parser and locker of the TokenTree), also, I remove the m_PoolTask, it was introduced to handle the priority header files, but now, we can already recursive to parse the included files, so I don't think m_PoolTask is needed any longer. Also, my local branch has many other issues I need to fix, so it will take a long time before I publish the patches  ;)


EDIT:

--- Quote from: Huki on June 16, 2014, 06:07:37 pm ---A bugfix for crash when trying to cancel the ongoing project parsing (to reparse, quit CB, etc). Easiest way to reproduce is to select "Project -> Reparse current project" twice successively (or once before the initial parsing has finished). It's in fact a mutex deadlock, so here is the patch:
...

--- End quote ---
I try to reproduce this bug, but sorry I can't reproduce the dead lock in rev9807, under Window XP. I try the two method:
method one:

--- Code: ---1, load a project, so the parsing started.
2, right click on the project, and select Peparse current project
3, no crash happened

--- End code ---

method two:

--- Code: ---1, right click on the project, and select Peparse current project
2, right click on the project, and select Peparse current project again
3, no crash happened

--- End code ---

Can you give some more detailed explanation? Thanks.

White-Tiger:
I guess he fixed what I were talking about back here: http://forums.codeblocks.org/index.php/topic,18044.msg124560.html#msg124560
You never fixed it IIRC... probably because you couldn't reproduce it.. (I couldn't get a proper stack trace)

But it's been a while since I had a freeze like that... I'm carefully with closing projects :P (and I've got a new PC now... so it's even more unlikely)

ollydbg:

--- Quote from: White-Tiger on June 17, 2014, 10:26:41 am ---I guess he fixed what I were talking about back here: http://forums.codeblocks.org/index.php/topic,18044.msg124560.html#msg124560
You never fixed it IIRC... probably because you couldn't reproduce it.. (I couldn't get a proper stack trace)

But it's been a while since I had a freeze like that... I'm carefully with closing projects :P (and I've got a new PC now... so it's even more unlikely)

--- End quote ---
Thanks for the remind, yes, CC sometimes just hangs on my system too (I use a notepad which bought in 2009, CC hangs when loading a large project, it always hang at the first time I load the project, not the second time), It looks like other devs don't have such hang issue.  :(

ollydbg:
The parser handling is a mass, see a belief draft diagram

--- Code: ---CodeCompletion receive Workspace changed (mostly because project loaded finishes)
->NativeParser::CreateParser for the active project
  ->new Parser
  ->DoFullParsing
    ->Fill Parser's macro definition(from compiler and from project setting)
    ->Fill Parser's file list need to parse, kick the batch timer
   
Parser receive batch timer event
->new ParserThreadedTask (this task will executed in thread pool)
->send Parse Start Event!

ParserThreadedTask is executed
->parse the macro definition
->new ParserThread for each file in file list
->put ParserThread in thread queue

Thread pool finish running tasks:
->do one of below
  *CASE1: if thread queue is not empty, copy to thread pool, run
  *CASE2: if file list is not empty, kick the batch timer
  *CASE3: if macro definition is not empty, kick the batch timer
  *CASE4: non of the above cases, send Parse Finish Event!


--- End code ---

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version