Author Topic: The 08 February 2014 build (9639) is out.  (Read 58167 times)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: The 08 February 2014 build (9639) is out.
« Reply #30 on: February 12, 2014, 03:08:36 pm »
@morten, revert rev9369 can avoid the hang issue, so I'm going to commit the change now. (This is a workaround, not a true fix).

@ollydbg: reverting r9638 fixes this issue too!

I can confirm that r9639 has this issue while r9619 works flawlessly. Between this revisions, CC-related changes are r9636 and r9638.

- osdt
Dear osdt, thanks for the reply. Rev9638 is a solid fix for rev 9601, because when in rev 9601, I leave a logic error.

See what rev9638 does:
Code
 src/plugins/codecompletion/parser/tokenizer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/plugins/codecompletion/parser/tokenizer.cpp b/src/plugins/codecompletion/parser/tokenizer.cpp
index 6761574..25b0a6d 100644
--- a/src/plugins/codecompletion/parser/tokenizer.cpp
+++ b/src/plugins/codecompletion/parser/tokenizer.cpp
@@ -1985,7 +1985,7 @@ bool Tokenizer::GetMacroExpendedText(const Token* tk, wxString& expandedText)
 
     // sanity check if we have such macro definition that #define AAA(x,y) x+y+AAA
     // which means a macro name is exists in its definition, which will cause a infinite expansion loop
-    if (tk->m_FullType.Find(tk->m_Name) ==wxNOT_FOUND)
+    if (tk->m_FullType.Find(tk->m_Name) != wxNOT_FOUND)
         return false;
 
     // Now, tk is a function like macro definition we are going to expand, it's m_Args contains the

You see, if you leave the  if (tk->m_FullType.Find(tk->m_Name) ==wxNOT_FOUND) here, than you just disable the macro expansion feature now, so it should definitely be  != for this sanity check.

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 ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: The 08 February 2014 build (9639) is out.
« Reply #31 on: February 12, 2014, 03:36:46 pm »
@morten, revert rev9369 can avoid the hang issue, so I'm going to commit the change now. (This is a workaround, not a true fix).
Done in r9647, sorry to all who have the hang issue.
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 osdt

  • Multiple posting newcomer
  • *
  • Posts: 63
Re: The 08 February 2014 build (9639) is out.
« Reply #32 on: February 12, 2014, 03:37:46 pm »
Dear osdt, thanks for the reply. Rev9638 is a solid fix for rev 9601, because when in rev 9601, I leave a logic error.

Ok. I was just bitsecting this issue and came across r9638.

-osdt

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: The 08 February 2014 build (9639) is out.
« Reply #33 on: February 13, 2014, 05:36:24 pm »
I debugged a little, I think it was a regression after a patch by Huki,
So, bug is here:
Code
        // Check whether a ReplaceBufferText() was done in DoGetToken().
        // We assume m_Undo... have already been reset in ReplaceBufferText().
        if (m_IsReplaceParsing && savedReplaceCount != (int)m_RepeatReplaceCount)
        {
            m_TokenIndex             = m_UndoTokenIndex;                       //*************bug here******************
            m_LineNumber             = m_UndoLineNumber;
            m_NestLevel              = m_UndoNestLevel;
        }
m_TokenIndex is always remain/reset to the same value, so Tokenizer won't go forward any more.

Are we dealing with really complex nested macros in wx3.0? I'm assuming that is the problem...
A quick explanation of my patch for PeekToken(): the goal is to reset m_TokenIndex every time a macro replacement occurs inside ReplaceBufferText(). So we use the test "if (m_IsReplaceParsing && savedReplaceCount != (int)m_RepeatReplaceCount)", to see if m_RepeatReplaceCount has increased after calling DoGetToken(). This should be safe because there is a limit on the number of nested macro replacements allowed (s_MaxRepeatReplaceCount), so infinite loop is impossible.

But seeing the concerned code, I think the problem is obvious: (in tokenizer.cpp)
Code
bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken)
{
    if (target.IsEmpty())
        return false;

    if (m_IsReplaceParsing && ++m_RepeatReplaceCount > s_MaxRepeatReplaceCount)
    {
        m_TokenIndex = m_BufferLen - m_FirstRemainingLength;
        m_PeekAvailable = false;
        SkipToEOL(false);
        return false;
    }

    ...


We can see that the m_RepeatReplaceCount will keep increasing even after s_MaxRepeatReplaceCount is reached, even though no more replacement occurs. We should change it to something like this:
Code
bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken)
{
    if (target.IsEmpty())
        return false;

    if (m_IsReplaceParsing && m_RepeatReplaceCount >= s_MaxRepeatReplaceCount)
        return false;
 
    ++m_RepeatReplaceCount;
    
    ...


@ollydbg: Does my patch still cause the hang with the above fix?
« Last Edit: February 13, 2014, 08:05:06 pm by Huki »

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: The 08 February 2014 build (9639) is out.
« Reply #34 on: February 14, 2014, 02:49:32 am »
...
@ollydbg: Does my patch still cause the hang with the above fix?
@Huki, many thanks, fairly clear explanation, I just test the code and no hang now.
This is the code I use:
Code
bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken)
{
    if (target.IsEmpty())
        return false;

    if (m_IsReplaceParsing)
    {
        if (m_RepeatReplaceCount >= s_MaxRepeatReplaceCount)
        {
            m_TokenIndex = m_BufferLen - m_FirstRemainingLength;
            m_PeekAvailable = false;
            SkipToEOL(false);
            return false;
        }
        else
            ++m_RepeatReplaceCount;
    }

.....

BTW: I originally thought the test condition in PeekToken() can be changed to

Quote
       if (m_IsReplaceParsing && savedReplaceCount < (int)m_RepeatReplaceCount)

Because in some cases, m_RepeatReplaceCount will reset to zero, but finally I found that is not necessary, because both m_IsReplaceParsing and m_RepeatReplaceCount will be reset to zero in the same time.

Code
    if (m_FirstRemainingLength != 0 && m_BufferLen - m_FirstRemainingLength < m_TokenIndex)
    {
        m_FirstRemainingLength = 0;
        m_IsReplaceParsing = false;
        m_RepeatReplaceCount = 0;
    }

BTW2: Why we need two variables? I think m_IsReplaceParsing is a redundant variable of m_RepeatReplaceCount. Checking all the source code, I realize that
Code
RepeatReplaceCount > 0 means m_IsReplaceParsing == true
RepeatReplaceCount == 0 means m_IsReplaceParsing == false
So, if we remove the m_IsReplaceParsing, we can change the test to
Quote
       if (savedReplaceCount < m_RepeatReplaceCount)
But the code to initialize savedReplaceCount can be simply
Code
size_t savedReplaceCount = m_RepeatReplaceCount;

Then
Code
if (m_IsReplaceParsing)
can change to
Code
if (m_RepeatReplaceCount > 0)

Also need to adjust the code below
Code
    // Set replace parsing state, and save first replace token index
    if (!m_IsReplaceParsing)
    {
        m_FirstRemainingLength = m_BufferLen - m_TokenIndex;
        m_IsReplaceParsing = true;
    }

I will prepare two commits:
1, fix the hang issue as you suggest.
2, code refactoring by remove the m_IsReplaceParsing variable.

Finally, thanks for your help!


« Last Edit: February 14, 2014, 02:51:36 am by ollydbg »
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 Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: The 08 February 2014 build (9639) is out.
« Reply #35 on: February 15, 2014, 03:32:48 am »
Glad to know the hang was fixed. :)

I will prepare two commits:
1, fix the hang issue as you suggest.
2, code refactoring by remove the m_IsReplaceParsing variable.
I checked both commits, it's ok for me.. and yes, removing the redundant m_IsReplaceParsing var is a nice improvement.

Offline stahta01

  • Lives here!
  • ****
  • Posts: 7582
    • My Best Post
Re: The 08 February 2014 build (9639) is out.
« Reply #36 on: February 18, 2014, 05:05:35 pm »
Can someone apply this short fix?

Patch that get rid of the Install Plugin error on Windows; after 2 hours of trying semi-random stuff I found a fix.
Error message was "One or more plugins were not installed successfully".

Tim S.

Code
Index: src/sdk/pluginmanager.cpp
===================================================================
--- src/sdk/pluginmanager.cpp (revision 9653)
+++ src/sdk/pluginmanager.cpp (working copy)
@@ -282,7 +282,7 @@
         settingsOnName.Remove(0, 3);
     if (!platform::windows && settingsOffName.StartsWith(_T("lib")))
         settingsOffName.Remove(0, 3);
-    wxString pluginFilename = pluginDir + _T('/') + localName;
+    wxString pluginFilename = UnixFilename(pluginDir + _T('/') + localName);
 //    Manager::Get()->GetLogManager()->DebugLog(F(_T("Plugin filename: ") + pluginFilename));
 //    Manager::Get()->GetLogManager()->DebugLog(F(_T("Plugin resources: ") + ConfigManager::GetDataFolder() + _T('/') + resourceName));
 

Thanks.

Tim S.
C Programmer working to learn more about C++ and Git.
On Windows 7 64 bit and Windows 10 64 bit.
--
When in doubt, read the CB WiKi FAQ. http://wiki.codeblocks.org

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: The 08 February 2014 build (9639) is out.
« Reply #37 on: February 18, 2014, 05:17:44 pm »
Thanks.
I would, but currently I've no access to SVN. I've noted it down... stay tuned...
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