Author Topic: parser test rev 7157 error?  (Read 24400 times)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
parser test rev 7157 error?
« on: May 24, 2011, 09:09:27 am »
Hi, morten, I found that in the rev 7157, you enabled the parsing include files. but look at this:
Code
bool ParserThread::Parse()
{
    wxCriticalSectionLocker locker(g_ParserThreadCritical);

    if (!IS_ALIVE || !InitTokenizer())
        return false;

    TRACE(_T("Parse() : Parsing '%s'"), m_Filename.wx_str());

    bool result = false;
    m_ParsingTypedef = false;

    do
    {
        if (!m_TokensTree || !m_Tokenizer.IsOK())
            break;

        if (!m_Options.useBuffer) // Parse a file
        {
            wxCriticalSectionLocker locker(s_TokensTreeCritical);
            m_FileIdx = m_TokensTree->ReserveFileForParsing(m_Filename);
            if (!m_FileIdx)
                break;
        }

        DoParse();

        if (!m_Options.useBuffer) // Parsing a file
        {
            wxCriticalSectionLocker locker(s_TokensTreeCritical);
            m_TokensTree->FlagFileAsParsed(m_Filename);
        }

        result = true;
    }
    while (false);

    return result;
}

This function will be called recursively, but why the first statement:
Code
wxCriticalSectionLocker locker(g_ParserThreadCritical);

It seems this locker should block the recursive call, but in-fact, it doesn't. I test under mingw+windows. That's was quite strange...

Any ideas?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: parser test rev 7157 error?
« Reply #1 on: May 24, 2011, 01:18:35 pm »
It seems this locker should block the recursive call, but in-fact, it doesn't.
Any ideas?
I am aware of this and no, I havent' looked into it. I could imagine that another (wrong) mutex is locked, or, that the mutex is created multiple times. I've marked another (possible) crash anyways, as this still is a work-in-progress. My "masterplan" was to collect the included files and parse them one-by-one to avoid this and (most important) the other crash.

But as it works as it is now I wanted to do an interim-commit, so people see what's happening and we are avoiding conflicts. This is a test project after all, so it's not critical. My intention is to fix the bug with the parser being stalled.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: parser test rev 7157 error?
« Reply #2 on: May 26, 2011, 07:50:11 am »
This function will be called recursively,
No, The DoParse will be called recursively, not is Parse.

It seems this locker should block the recursive call, but in-fact, it doesn't. I test under mingw+windows. That's was quite strange...
A source file, *.h, or *.c/cpp, should call Parse() once.
This locker make them parsed one by one.
« Last Edit: May 26, 2011, 07:51:55 am by Loaden »

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: parser test rev 7157 error?
« Reply #3 on: May 26, 2011, 07:53:38 am »
I've marked another (possible) crash anyways, as this still is a work-in-progress.
Sorry, I can't solved the random crash.
I am very happy to see you watched the same issue. :D

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: parser test rev 7157 error?
« Reply #4 on: May 26, 2011, 08:11:14 am »
Here is my test example:
let the test.h be like below:
Code
int aaaaa;
float bbbbb;
#include <a.h>
#include <b.h>

create "a.h" and "b.h" along with "test.h". (in the same folder)

here is a.h
Code
int in_a_h;
#include "b.h"

Here is b.h
Code
int in_b_h;

Then debug the parsertest project, see the screenshot:


Why does the function "Parse" recursively called???
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: parser test rev 7157 error?
« Reply #5 on: May 26, 2011, 08:19:28 am »
One Parser per one thread.
So in here, there has three thread was started.
We use threadpool for batch parsing.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: parser test rev 7157 error?
« Reply #6 on: May 26, 2011, 08:20:49 am »
One Parser per one thread.
So in here, there has three thread was started.
We use threadpool for batch parsing.
but g_ParserThreadCritical is a global variable, it only allowed one entry.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: parser test rev 7157 error?
« Reply #7 on: May 26, 2011, 08:46:36 am »
Why does the function "Parse" recursively called???
Your example is OK. The implementation of ParserTest provides the global variable "g_ParserCritical". As ParserTest is intatiated several times (inspect the addresses in your debug - they differ) the locker is also created multiple times. And that is OK, because the lock shall apply just the one object itself.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: parser test rev 7157 error?
« Reply #8 on: May 26, 2011, 08:57:08 am »
One Parser per one thread.
So in here, there has three thread was started.
We use threadpool for batch parsing.
but g_ParserThreadCritical is a global variable, it only allowed one entry.
Right, That's the final want.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: parser test rev 7157 error?
« Reply #9 on: May 26, 2011, 10:47:38 am »
One Parser per one thread.
So in here, there has three thread was started.
We use threadpool for batch parsing.
@loaden, In the parsertest.cbp, we do NOT use any kind of threadpool for batch parsing, do we? The parsertest project does not use multithread, does it?

As ParserTest is intatiated several times (inspect the addresses in your debug - they differ) the locker is also created multiple times. And that is OK, because the lock shall apply just the one object itself.
Yes, they are many different locker instances, but all the instances use the same "static wxCriticalSection g_ParserThreadCritical;"
I have never known about this is a correct thing before, I will try to learn more things about multi-thread programming.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: parser test rev 7157 error?
« Reply #10 on: May 27, 2011, 10:30:35 am »
@loaden, In the parsertest.cbp, we do NOT use any kind of threadpool for batch parsing, do we? The parsertest project does not use multithread, does it?
In ParserTest, we are not use threadpool.
But in CB, we use threadpool, see here.
Quote
bool Parser::Parse(const wxString& bufferOrFilename, bool isLocal, ParserThreadOptions& opts)
{
    bool result = false;
    do
    {
        if (!opts.useBuffer)
        {
            wxCriticalSectionLocker locker(s_TokensTreeCritical);

            bool canparse = !m_TokensTree->IsFileParsed(bufferOrFilename);
            if (canparse)
                canparse = m_TokensTree->ReserveFileForParsing(bufferOrFilename, true) != 0;

            if (!canparse)
            {
               if (opts.loader) // if a loader is already open at this point, the caller must clean it up
                   Manager::Get()->GetLogManager()->DebugLog(_T("Parse() : CodeCompletion Plugin: FileLoader memory leak likely while loading file ")+bufferOrFilename);
               break;
            }

            if (!opts.loader) // this should always be true (memory will leak if a loader has already been initialized before this point)
                opts.loader = Manager::Get()->GetFileManager()->Load(bufferOrFilename, m_NeedsReparse);
        }

        TRACE(_T("Parse() : Creating task for: %s"), bufferOrFilename.wx_str());
        ParserThread* thread = new ParserThread(this, bufferOrFilename, isLocal, opts, m_TokensTree);
        bool doParseNow = opts.useBuffer;
#if CC_PARSER_PROFILE_TEST
        doParseNow = true;
#endif
        //if we are parsing a memory buffer or Parser is under Profile
        // CC_PARSER_PROFILE_TEST is defined as 1), then just call Parse()
        // directly and thread pool is NOT used.
        // Otherwise, we are parsing a local file, so the thread pool is used.
        // The ParserThread generated was pushed to the memory pool.

        if (doParseNow)
        {
            result = thread->Parse();
            LinkInheritance(true);
            delete thread;
            break;
        }

        TRACE(_T("Parse() : Parsing %s"), bufferOrFilename.wx_str());

        if (!m_IsPriority)
        {
            TRACE(_T("Parse() : Parallel Parsing %s"), bufferOrFilename.wx_str());

            // Add a task for all project files
            if (m_IsFirstBatch)
            {
                m_IsFirstBatch = false;
                m_PoolTask.push(PTVector());
            }

            if (m_IsParsing)
                m_Pool.AddTask(thread, true);
            else
                m_PoolTask.back().push_back(thread);
        }
        else if (m_IsPriority)
        {
            if (isLocal) // Parsing priority files
            {
                TRACE(_T("Parse() : Parsing priority header, %s"), bufferOrFilename.wx_str());
                result = thread->Parse();
                delete thread;
                break;
            }
            else // Add task when parsing priority files
            {
                TRACE(_T("Parse() : Add task for priority header, %s"), bufferOrFilename.wx_str());
                m_PoolTask.push(PTVector());
                m_PoolTask.back().push_back(thread);
            }
        }

        result = true;
    }
    while (false);

    return result;
}

One header/source file per one ParserThread.
And operating them one by one.

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: parser test rev 7157 error?
« Reply #11 on: May 27, 2011, 10:34:13 am »
So, you can create a stand project, and debug again.
This case only happen with parsertest project.

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: parser test rev 7157 error?
« Reply #12 on: May 27, 2011, 10:44:50 am »
So, you can create a stand project, and debug again.
This case only happen with parsertest project.
OMG, Seems still have some problem.
Quote
bool Parser::Parse(const wxString& bufferOrFilename, bool isLocal, ParserThreadOptions& opts)
{
    bool result = false;
    do
    {
        if (!opts.useBuffer)
        {
            wxCriticalSectionLocker locker(s_TokensTreeCritical);

            bool canparse = !m_TokensTree->IsFileParsed(bufferOrFilename);
            if (canparse)
                canparse = m_TokensTree->ReserveFileForParsing(bufferOrFilename, true) != 0;

            if (!canparse)
            {
               if (opts.loader) // if a loader is already open at this point, the caller must clean it up
                   Manager::Get()->GetLogManager()->DebugLog(_T("Parse() : CodeCompletion Plugin: FileLoader memory leak likely while loading file ")+bufferOrFilename);
               break;
            }

            if (!opts.loader) // this should always be true (memory will leak if a loader has already been initialized before this point)
                opts.loader = Manager::Get()->GetFileManager()->Load(bufferOrFilename, m_NeedsReparse);
        }

        TRACE(_T("Parse() : Creating task for: %s"), bufferOrFilename.wx_str());
        ParserThread* thread = new ParserThread(this, bufferOrFilename, isLocal, opts, m_TokensTree);
        bool doParseNow = opts.useBuffer;
#if CC_PARSER_PROFILE_TEST
        doParseNow = true;
#endif
        //if we are parsing a memory buffer or Parser is under Profile
        // CC_PARSER_PROFILE_TEST is defined as 1), then just call Parse()
        // directly and thread pool is NOT used.
        // Otherwise, we are parsing a local file, so the thread pool is used.
        // The ParserThread generated was pushed to the memory pool.

        if (doParseNow)
        {
            result = thread->Parse();
            LinkInheritance(true);
            delete thread;
            break;
        }

        TRACE(_T("Parse() : Parsing %s"), bufferOrFilename.wx_str());

        if (!m_IsPriority)
        {
            TRACE(_T("Parse() : Parallel Parsing %s"), bufferOrFilename.wx_str());

            // Add a task for all project files
            if (m_IsFirstBatch)
            {
                m_IsFirstBatch = false;
                m_PoolTask.push(PTVector());
            }

            if (m_IsParsing)
                m_Pool.AddTask(thread, true);
            else
                m_PoolTask.back().push_back(thread);
        }
        else if (m_IsPriority)
        {
            if (isLocal) // Parsing priority files
            {
                TRACE(_T("Parse() : Parsing priority header, %s"), bufferOrFilename.wx_str());
                result = thread->Parse(); // ### HERE, some thing is wrong!
                delete thread;
                break;
            }

            else // Add task when parsing priority files
            {
                TRACE(_T("Parse() : Add task for priority header, %s"), bufferOrFilename.wx_str());
                m_PoolTask.push(PTVector());
                m_PoolTask.back().push_back(thread);
            }
        }

        result = true;
    }
    while (false);

    return result;
}
:(

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: parser test rev 7157 error?
« Reply #13 on: May 27, 2011, 10:51:27 am »
@loaden, let's focus our discussion on the ParserTest project.
As there is no multi-thread running thread pool, all the parsing is done in the main thread. and there are some recursive called if the parser meets a #include directive. My puzzle is still the same, as it is only one global critical section, why it cam create different critical section lockers? and it seems these sections were entered several times???? :(
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: parser test rev 7157 error?
« Reply #14 on: May 27, 2011, 11:13:47 am »
I'm removed the locker in my local commit (git-svn).
And commit them on tonight.