Remove the usage of "null backend"
ClosedPublic

Authored by asemke on Sep 16 2019, 7:40 PM.

Details

Summary

In b45af5e4de0cf283bb06544f367be9aa242e90a3 the NullBackend class was removed. However, there is still some logic in the code making use of this "backend" which crashes Cantor when opening of a project file at the moment. This patch refactors the logic a bit and removes the remaining usage of the null backend completely.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
asemke created this revision.Sep 16 2019, 7:40 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptSep 16 2019, 7:40 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
asemke requested review of this revision.Sep 16 2019, 7:40 PM
sirgienko requested changes to this revision.Sep 17 2019, 10:11 AM
QDEBUG : WorksheetTest::testMathRender() PythonSession(0x5637c2e29f80) Cantor::VariableManagementExtension(0x5637c272c020, name = "VariableManagementExtension")
QDEBUG : WorksheetTest::testMathRender() PythonHighlighter constructor
FAIL!  : WorksheetTest::testMathRender() Compared values are not the same
   Actual   (rendered): 0
   Expected (true)    : 1
   Loc: [/home/mmmm1998/Документы/Репозитории/cantor/src/test/worksheet_test.cpp(6887)]
QDEBUG : WorksheetTest::testMathRender2() PythonSession(0x5637c3250fb0) Cantor::VariableManagementExtension(0x5637c272c020, name = "VariableManagementExtension")
QDEBUG : WorksheetTest::testMathRender2() PythonHighlighter constructor
FAIL!  : WorksheetTest::testMathRender2() Compared values are not the same
   Actual   (rendered): 0
   Expected (true)    : 1
   Loc: [/home/mmmm1998/Документы/Репозитории/cantor/src/test/worksheet_test.cpp(6920)]

I don't fully understand, why the math render tests start to fail, but this occurs only with this patch, so the reason is the patch, i think.
So, we need additional work on this patch.

src/cantor.cpp
188–190

dynamic_cast without pointer validation - bad practise. Better use static_cast or check the poiter

This revision now requires changes to proceed.Sep 17 2019, 10:11 AM
asemke updated this revision to Diff 67827.Oct 13 2019, 8:40 AM

This patch seems to cause somewhat different timings in testworkseet which leads to a crash and wrong results. The problems seems to be because of parallelized execution of the tests here which is not completely clear. The new diff

  • avoids the crash
  • increases the wait time in the math renderer to get the proper result
asemke marked an inline comment as done.Oct 13 2019, 8:40 AM
asemke added a comment.EditedOct 13 2019, 8:54 AM

I don't fully understand, why the math render tests start to fail, but this occurs only with this patch, so the reason is the patch, i think.
So, we need additional work on this patch.

Multiple tests are executed in parallel here which I don't completely understand yet. The crash is in KColorScheme in MathRenderer

#0  0x00007fffef9f84ba in QObject::property(char const*) const () at /usr/lib64/libQt5Core.so.5
#1  0x00007ffff2f20d57 in  () at /usr/lib64/libKF5ConfigWidgets.so.5
#2  0x00007ffff2f232a0 in KColorScheme::KColorScheme(QPalette::ColorGroup, KColorScheme::ColorSet, QExplicitlySharedDataPointer<KSharedConfig>) ()
    at /usr/lib64/libKF5ConfigWidgets.so.5
#3  0x00000000004953a6 in MathRenderTask::run() ()
#4  0x00007fffef7e8202 in  () at /usr/lib64/libQt5Core.so.5
#5  0x00007fffef7e48e2 in  () at /usr/lib64/libQt5Core.so.5
#6  0x00007fffee883569 in start_thread () at /lib64/libpthread.so.0
#7  0x00007fffeeb92a2f in clone () at /lib64/libc.so.6

which can be avoided by the change I did in the second version of the patch in MathRendererTask. After the problem with the crash is solved, two tests are still failing

FAIL!  : WorksheetTest::testMathRender() Compared values are not the same
   Actual   (rendered): 0
   Expected (true)    : 1
   Loc: [/home/alex/Projekte/cantor/src/test/worksheet_test.cpp(6888)]
FAIL!  : WorksheetTest::testMathRender2() Compared values are not the same
   Actual   (rendered): 0
   Expected (true)    : 1
   Loc: [/home/alex/Projekte/cantor/src/test/worksheet_test.cpp(6921)]
PASS   : WorksheetTest::cleanupTestCase()

This is fixed, at least on my computer, by setting the waiting time in these tests to 5 seconds.

Without the new change in MathRenderTask and without increasing the waiting time to 5 seconds, these two tests are not failing (no crash, no wrong results) if they are executed alone. The problems occur only if all tests in testworksheet are executed and when there is this parallelization of the test execution. I don't think all these problems are directly caused by my patch. This code just leads to somewhat different timings in the execution maybe which trigger then the problems we observe.

After adddinп

qDebug()<<"MathRenderTask::run " << m_jobId;

in MathRenderTask::run() I get the following output for these two tests:

QDEBUG : WorksheetTest::testMathRender() PythonSession(0xe70940) Cantor::VariableManagementExtension(0xd22300, name = "VariableManagementExtension")
QDEBUG : WorksheetTest::testMathRender() PythonHighlighter constructor
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  9
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  10
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  11
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  12
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  13
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  6
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  7
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  8
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  9
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  10
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  11
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  12
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  6
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  7
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  8
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  9
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  10
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  11
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  12
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  13
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  14
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  15
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  16
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  17
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  6
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  7
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  8
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  9
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  10
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  6
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  8
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  7
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  9
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  6
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  7
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  8
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  9
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  10
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  11
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  12
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  13
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  14
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  15
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  16
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  6
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  7
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  8
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  9
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  10
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  6
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  7
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  6
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  7
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  2
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  3
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  4
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  5
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  6
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  7
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  8
QDEBUG : WorksheetTest::testMathRender() MathRenderTask::run  1
QDEBUG : WorksheetTest::testMathRender() result  1    true
PASS   : WorksheetTest::testMathRender()
QDEBUG : WorksheetTest::testMathRender2() PythonSession(0xebfe60) Cantor::VariableManagementExtension(0xd22300, name = "VariableManagementExtension")
QDEBUG : WorksheetTest::testMathRender2() PythonHighlighter constructor
QDEBUG : WorksheetTest::testMathRender2() MathRenderTask::run  1
PASS   : WorksheetTest::testMathRender2()
PASS   : WorksheetTest::cleanupTestCase()
Totals: 17 passed, 0 failed, 0 skipped, 0 blacklisted, 18691ms
********* Finished testing of WorksheetTest *********

Even though we have only one single entry to render in testMathRender(), there are multiple render tasks triggered here. We need to understand this. But again, this all seems to be an issue somewhere in MathRender, MathRenderTask and/or WorksheetTest.

Even though we have only one single entry to render in testMathRender(), there are multiple render tasks triggered here. We need to understand this. But again, this all seems to be an issue somewhere in MathRender, MathRenderTask and/or WorksheetTest.

I see. So, I think, we should revert time of waiting change (from 5 sec to 1 sec) and accept this patch.

asemke updated this revision to Diff 67830.Oct 13 2019, 9:34 AM

Removed the increase of the waiting time. This problem needs to be addressed differently, but outside of this patch.

sirgienko accepted this revision.Oct 13 2019, 9:35 AM
This revision is now accepted and ready to land.Oct 13 2019, 9:35 AM