Infinite loop in CodeEditorComponent

General JUCE discussion
1

I have a window with a CodeEditorComponent with an associated CodeDocument). It reads and displays files. If i open a new document and replacing previous text (using CodeDocument::loadFromStream()), it will hang IF i previously scrolled the old document to a position further down than what is available in the new document it seems (ie., the new document is smaller than the old).

The infinite loop is here:


            for (;;)
            {
                codeTokeniser->readNextToken (*t);
                if (t->getLine() >= targetLine)
                    break;
                if (t->isEOF())
                    return;
            }

inside "void CodeEditorComponent::updateCachedIterators (int maxLineNum)". It is called from scrollbarMoved->scrollToLineInternal iirc.

The code is using the standard CPlusPlusTokenizer. Stepping through the first couple of lines:


    template <typename Iterator>
    static int readNextToken (Iterator& source)
    {
        source.skipWhitespace();
        const juce_wchar firstChar = source.peekNextChar();

"firstChar" is always set to zero ('\0'), from which the tokenizer returns tokenType_error. Even if i wrote a custom tokenizer, there's no way to report the error back (return value isn't checked) beyond throwing an exception and crashing the application.

 

e: problem temporarily solved by calling CodeEditorComponent::ScrollToLine(0) before loading new text. Shouldn't this be default behaviour?

2

I'm struggling to see how that could happen without isEOF() returning true.. Have you got a code-snippet that makes it happen?

3

Maybe i can create an example later, when i have more time.. It is consistently reproducable btw.

Here's a debug output that might help you:

 

HiPXYe4.png953×828

 

I dont know exactly how it ended there (i just paused and stepped in after the app hanged), but it seems that all data is 'valid', ie. nothing is corrupted and the current line is in bounds. The charpointer however, is zero ( = '\0') - the pointer is not!

What happens next is, that t->isEOF() returns false (it only checks for out of bounds lines and NULL charPointer(), ie. not for EOF or nulltermination - is this intended?).

Meanwhile, the loop continues, and the CPlusPlusTokenizer, after encountering the nulltermination never increments the char pointer, only peeks at it and returns error. This is roughly what happens:

for (;;)
{
    codeTokeniser->readNextToken(*t):
        -> static int readNextToken(Iterator& source):
                source.skipWhitespace(); // does nothing
                const juce_wchar firstChar = source.peekNextChar(); // gets a zero
                switch (firstChar)
                {
                    case 0:
                        break; // breaks here, and
                }
                return CPlusPlusCodeTokeniser::tokenType_error; // returns error
        <-
    if (t->getLine() >= targetLine) // this check is passed
        break;
    if (t->isEOF()) // isEOF returns false, because '\0' is not considered EOF
        return;
    // loop continues
}


​Next loop, the character pointer is never incremented, were still on the same line and isEOF() returns false yet again.
 

4

Yeah, the bit I don't understand is that CodeDocument::Iterator::nextChar() is designed to always set charPointer to null when it reaches the EOF, so it should never get into a state where charPointer is a valid pointer that points to a 0...

Would probably be easy for me to fix if I could just reproduce it and step through it.

5

Here's a sample component. This consistently works here (well, 'works' as in reproduces the bug).

HangEditor.h


#ifndef _HANGEDITOR_H
#define _HANGEDITOR_H

#include "../JuceLibraryCode/JuceHeader.h"

class HangEditor : public CodeEditorComponent
{
public:
    HangEditor();
    ~HangEditor();
    void induceHang();
    void loadInitialText();
private:
    // dont have to be static, but it's easiest in this case.
    static CodeDocument doc;
    static CPlusPlusCodeTokeniser tok;
};

#endif

HangEditor.cpp


#include "HangEditor.h"
#include <thread>
#include <chrono>

CPlusPlusCodeTokeniser HangEditor::tok;
CodeDocument HangEditor::doc;

HangEditor::HangEditor()
    : CodeEditorComponent(doc, &tok)
{
    setSize (500, 400);
    loadInitialText();
    // hang editor after 10 seconds
    // can call editor->induceHang() anywhere, instead of this. 
    std::thread(&HangEditor::induceHang, this).detach();
}

HangEditor::~HangEditor()
{
}

void HangEditor::induceHang()
{
    // wait some time
    // might want to erase this line if you're calling it directly (ie. not from another thread)
    std::this_thread::sleep_for(std::chrono::milliseconds(10000));
    File f(File::getCurrentWorkingDirectory().getFullPathName() + "test.txt");
    // erase previous lines
    f.deleteFile();
    if (ScopedPointer<FileOutputStream> s = f.createOutputStream())
    {
        for (int i = 0; i < 1; i++)
            s->writeText("the quick brown fox jumped over the lazy dog\n", false, false);
    }
    // now file should only contain 1 line
    // since were in another thread, aqcuire messagemanager
    // again, might wanna erase this if called from main thread.
    const MessageManagerLock mml;
    if (ScopedPointer<FileInputStream> s = f.createInputStream())
    {
        doc.loadFromStream(*s);
    }
}

void HangEditor::loadInitialText()
{
    File f(File::getCurrentWorkingDirectory().getFullPathName() + "test.txt");
    if (ScopedPointer<FileOutputStream> s = f.createOutputStream())
    {
        for (int i = 0; i < 100; i++)
            s->writeText("the quick brown fox jumped over the lazy dog\n", false, false);
    }
    if (ScopedPointer<FileInputStream> s = f.createInputStream())
    {
        doc.loadFromStream(*s);
        scrollToLine(100);
    }
}
6

Thanks - should be fixed now.

7

Cool, thanks.