<table><tr><td style="">rjvbb created this revision.<br />rjvbb added a reviewer: Frameworks.<br />rjvbb added a project: KTextEditor.<br />Restricted Application added projects: Kate, Frameworks.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D8544" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>REVISION SUMMARY</strong><div><p>The transition from QtScript to QML introduced a propensity to crashing somewhere deep in Qt (in the V4 JIT engine to be exact), at often unexpected moments while editing texts, for users of certain versions of Qt5. It seems these crashes do not occur with Qt 5.9.1 and newer, but not everyone can update (readily) to that version.</p>
<p>Upstream bug report: <a href="https://bugreports.qt.io/browse/QTBUG-63045" class="remarkup-link" target="_blank" rel="noreferrer">https://bugreports.qt.io/browse/QTBUG-63045</a></p>
<p>I have tried to trace the JavaScript expressions that trigger the crashes I've seen myself, come up with a fix or at least a suitable and acceptable workaround (see <a href="https://bugs.kde.org/show_bug.cgi?id=385413" class="remarkup-link" target="_blank" rel="noreferrer">https://bugs.kde.org/show_bug.cgi?id=385413</a>). This review is for a patch that contains a fix for a specific crash as well as a general workaround.</p>
<p>As far as I can tell the crashes I get (when hitting enter at the end of a line in documents using C style indentation) occur when unwinding the script interpreter stack, for instance when exiting from a <tt style="background: #ebebeb; font-size: 13px;">while</tt> loop (or the equivalent <tt style="background: #ebebeb; font-size: 13px;">for</tt> loop). This particular crash can be avoided by returning early from the procedure containing the loop, instead of exiting from the loop and returning via the shared return statement; see the patch to <tt style="background: #ebebeb; font-size: 13px;">cstyle.js</tt>.</p>
<p>Gentoo have come up with a blunt-force "solution": build QtDeclarative with the V4 JIT disabled. It works just as well to launch applications that are susceptible to the crash with the <tt style="background: #ebebeb; font-size: 13px;">QV4_FORCE_INTERPRETER</tt> env. variable set which has less undesirable effects but is also more cumbersome.<br />
My patch explores an even less invasive approach: it uses the env. variable to disable the JIT when KTextEditor scripts are loaded/parsed, resetting (or unsetting) the variable when the crucial operation is done. The env.var manipulation is done in a dedicated KateScript subclass and is a noop for Qt version 5.9.1 and up.</p>
<p>BUG: 385413</p></div></div><br /><div><strong>TEST PLAN</strong><div><p>Tested on Mac and Linux with Qt 5.8.0 . This works for me (read: I haven't seen any other crashes - yet!) but apparently does not prevent crashing with Qt 5.7.1 (see the Qt bug report referenced in the summary).</p>
<p>If necessary we can of course disable the JIT proactively in a KTextEditor initialiser routine (if possible reenabling it for plugins).</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R39 KTextEditor</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D8544" rel="noreferrer">https://phabricator.kde.org/D8544</a></div></div><br /><div><strong>AFFECTED FILES</strong><div><div>src/script/data/indentation/cstyle.js<br />
src/script/katecommandlinescript.cpp<br />
src/script/kateindentscript.cpp<br />
src/script/katescript.cpp<br />
src/script/katescript.h<br />
src/script/katescripthelpers.cpp</div></div></div><br /><div><strong>To: </strong>rjvbb, Frameworks<br /><strong>Cc: </strong>kde-frameworks-devel, kevinapavew, demsking, head7, cullmann, kfunk, sars, dhaumann<br /></div>