QSyntaxHighlighter rehighlight() ignores changes triggered from another thread

ribtoks

I have a spellchecking thread which fires spellcheck() signals from time to time which are connected to my highlighter's rehighlight() method. The latter sets the whole block to have red foreground.

This used to work in Qt 5.6.2 and ceased to work in newer versions. I hopelessly waited for it to get fixed in Qt 5.9.5, but it still does not work (neither in Windows 10, nor in OS X).

void SpellCheckErrorsHighlighter::highlightBlock(const QString &text) {
    if (!m_Worker->isOK()) { return; }

    this->setFormat(0, text.length(), QColor(0xff, 0, 0));
}

// ------

void MainModel::startChecking() {
    m_Worker = new SpellCheckWorker();
    QThread *thread = new QThread();
    m_Worker->moveToThread(thread);

    QObject::connect(thread, &QThread::started, m_Worker, &SpellCheckWorker::process);
    thread->start();
}

void MainModel::initNameHighlighting(QQuickTextDocument *document) {
    SpellCheckErrorsHighlighter *highlighter = new SpellCheckErrorsHighlighter(m_Worker, document->textDocument());
    QObject::connect(m_Worker, &SpellCheckWorker::spellcheck,
                     highlighter, &SpellCheckErrorsHighlighter::rehighlight);
}

// ------

void SpellCheckWorker::process() {
    while (1) {
        m_Counter++;
        m_IsOK = m_Counter % 7 == 0;
        QThread::sleep(1);
        emit spellcheck();
   }
}

// ------

TextEdit {
    id: titleTextInput
    width: paintedWidth > titleFlick.width ? paintedWidth : titleFlick.width
    height: titleFlick.height
    text: mainModel.name
    focus: true
    onTextChanged: mainModel.name = text

    Component.onCompleted: mainModel.initNameHighlighting(titleTextInput.textDocument)
    onCursorRectangleChanged: titleFlick.ensureVisible(cursorRectangle)
}

// -----

int main(int argc, char *argv[]) {
    QGuiApplication app(argc, argv);

    MainModel mainModel;
    mainModel.startChecking();

    QQmlApplicationEngine engine;

    QQmlContext *rootContext = engine.rootContext();
    rootContext->setContextProperty("mainModel", &mainModel);

    engine.load(QUrl(QStringLiteral("qrc:/main.qml")));

    return app.exec();
}

Small example which reproduces the problem could be obtained from here https://bitbucket.org/ribtoks/qt-highlighting-issue (in order to repro, type something in the input. rehighlight() will be triggered each 7 seconds from background thread)

Is there any way to get it working with workarounds? I need to keep spellchecking logic in the background thread so it's not possible to move it to main thread.

aha_1980

@ribtoks said in QSyntaxHighlighter rehighlight() ignores changes triggered from another thread:

I hopelessly waited for it to get fixed in Qt 5.9.5

Have you ever created a bug report on bugreports.qt.io ?

ribtoks

@aha_1980 said in QSyntaxHighlighter rehighlight() ignores changes triggered from another thread:

@ribtoks said in QSyntaxHighlighter rehighlight() ignores changes triggered from another thread:

I hopelessly waited for it to get fixed in Qt 5.9.5

Have you ever created a bug report on bugreports.qt.io ?

Of course I did. It's never fixed P2. Qt people never care about such bugs, they only care about fixing enterprise issues..
https://bugreports.qt.io/browse/QTBUG-65248

aha_1980

@ribtoks

The report was one week before christmas, so we effectively have three month since the report. I would not call that never.

Furthermore, if you have a commercial license, let the commercial support add the corresponding tag to the report. Also Qt developers need to pay their flat and get something to eat and drink.

If you don't have, there is more you can do. If 5.6.2 is the last working version, you can try to find the first broken version. Is it 5.7, 5.8, or 5.9? Or was it in a patch release?

You can even go a step further and do git bisect to find the first broken commit. That's all a lot of work that keeps the developer from developing (and fixing bugs).

And last but not least, you are the only watcher of this bug, so I guess it's just a corner case you have hit. There are other bugs that effect a lot of people and that are therefore with higher priority.

Thanks.

mrjj

Hi
Ran your bug report sample in 5.10.1
alt text

Is that the expected non working output ?

ribtoks

@aha_1980 said in QSyntaxHighlighter rehighlight() ignores changes triggered from another thread:

@ribtoks

The report was one week before christmas, so we effectively have three month since the report. I would not call that never.

Furthermore, if you have a commercial license, let the commercial support add the corresponding tag to the report. Also Qt developers need to pay their flat and get something to eat and drink.

If you don't have, there is more you can do. If 5.6.2 is the last working version, you can try to find the first broken version. Is it 5.7, 5.8, or 5.9? Or was it in a patch release?

You can even go a step further and do git bisect to find the first broken commit. That's all a lot of work that keeps the developer from developing (and fixing bugs).

And last but not least, you are the only watcher of this bug, so I guess it's just a corner case you have hit. There are other bugs that effect a lot of people and that are therefore with higher priority.

Thanks.

The bug first appeared in 5.6.3 and later. Obviously I don't have commercial license otherwise I wouldn't complain on forums but called support directly.

Are you kidding regarding git bisect? I cannot waste days rebuilding Qt all the time with different patches and then running my test against it by hand. It cannot be detected automatically, unfortunately.

Yes, I'm the only watcher, but I wouldn't call it such an edge case - to desire Qt to behave accordingly to specification (when you do setFormat() for the format to change).

I believe this should be very easy to fix for a person who is familiar with the codebase. Probably just description of the problem would be enough for such person, but there's also a working sample.

ribtoks

@mrjj said in QSyntaxHighlighter rehighlight() ignores changes triggered from another thread:

Hi
Ran your bug report sample in 5.10.1

Is that the expected non working output ?

Hi. I will check it in 5.10.1 tomorrow, thanks!
Sometimes if you manually put cursor into the TextEdit it gets updated and turns red, but it's not always like that.
If you can repro it consistently then maybe it's working in 5.10.1 (I wanted to LTS), I'll try tomorrow.

Christian Ehrlicher

I tested it and it looks like it works as expected - every 8 seconds the text is marked completely red when I type something.
But the way you check for isOK() is for sure not what you want. You want an atomic here. If you don't trust me, see for example here https://stackoverflow.com/questions/4557979/when-to-use-volatile-with-multi-threading

ribtoks

@Christian-Ehrlicher said in QSyntaxHighlighter rehighlight() ignores changes triggered from another thread:

I tested it and it looks like it works as expected - every 8 seconds the text is marked completely red when I type something.
But the way you check for isOK() is for sure not what you want. You want an atomic here. If you don't trust me, see for example here https://stackoverflow.com/questions/4557979/when-to-use-volatile-with-multi-threading

Hi @Christian-Ehrlicher
Thanks for trying it out. Could you please let me know it works as expected using Qt of which version?
Yeah, I know about atomics, this was just super dirty hack not to rehighlight each second. My real code does not have this garbage.

aha_1980

@ribtoks said in QSyntaxHighlighter rehighlight() ignores changes triggered from another thread:

The bug first appeared in 5.6.3 and later.

So it is a regression from 5.6.2 to 5.6.3? I wonder why this important information is not stated in the bug report.

Are you kidding regarding git bisect? I cannot waste days rebuilding Qt all the time with different patches and then running my test against it by hand. It cannot be detected automatically, unfortunately.

And therefore you expect others to search for the problem?

ribtoks

@aha_1980 I expect "others" who know this particular code to look at the code, not to run git bisect and test every other patch. I believe there's a person writing most of the QTextDocument and QTextDocumentPrivate and who knows how it works. I don't know how it works and it will probably take me quite some time, not mentioning time to setup build environment for Qt.

Qt 5.6.x was LTS where people continuously fix stuff, not break stuff which used to work before. And it's just ridiculous somebody assigned P2 "Critical" to the issue, but nobody cares to fix. I would understand if it was P6 and nobody cared.

Christian Ehrlicher

It works with 5.10.0 / linux for me.
btw: I don't see any significant changes in qsyntaxhighlighter.cpp for the last years

ribtoks

@Christian-Ehrlicher said in QSyntaxHighlighter rehighlight() ignores changes triggered from another thread:

It works with 5.10.0 / linux for me.
btw: I don't see any significant changes in qsyntaxhighlighter.cpp for the last years

The change could be in QTextDocument or TextEdit (the end result is that TextEdit does not update the highlighting). I never said the bug is inside QSyntaxHighlighter. Thanks for checking it btw!

For me it does not work on macOS and Windows.

SGaist

Hi,

Got text hightlighted on macOS from time to time on macOS with 5.10 and 5.11 self-built.

ribtoks

@SGaist said in QSyntaxHighlighter rehighlight() ignores changes triggered from another thread:

Hi,

from time to time on macOS

Hello. Yes, that's the problem, "from time to time". Thanks for trying it out btw!