| apol | |
| mwolff |
| KDevelop |
a bug in kLineEdit causes spurious emission of textEdited signals when calling setText().
This in turn causes spurious calls of compilerEdited() when selecting a compiler in the compilers window,
which leads to a segfault (e.g. this bug https://bugs.kde.org/show_bug.cgi?id=373004) and I think some duplicates also.
I have submitted also a patch to kcompletion, https://phabricator.kde.org/D9808
This is a workaround for those systems until that is fixed
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
I don't understand how this fixes the crash. Can you extend the bug report with a better backtrace that contains more symbols? Also, the bug report looks like it's due to a queued signal emission. Which signal is that? And it looks like something withing the kdevplatform project library, how is that related to the compiler selection?
I will try and produce a more extensive backtrace, yes. The problem amounts to this:
because kLineEdit emits a spurious "textChanged" signal when calling setText, this causes "compilerEdited" to be called when in fact only the compiler selection has changed, rather than the path edited (in _addition_ to compilerSelected()). the setText() call in question is precisely the one in compilerSelected().
This extra call of compilerEdited then fires the assert
Q_ASSERT(!indexes.isEmpty())
because at this stage, the TreeView has not yet selected the new compiler, resulting in an empty selection index.
The same problem happens when moving away from the selection, because the spurious textEdited signal also comes from the clear() call in enableItems(false);
So just remove the assert - instead return early when indexes.isEmpty(). That way we don't need to mess with the signal blocker and the code behaves as intended. And you remove code instead of adding more.
improved backtrace with all symbols now attached to the bug report:
https://bugs.kde.org/show_bug.cgi?id=373004
I agree that fixes the crash, in fact that was the first fix I tried.
However, compilerEdited is still being spuriously called, and the underlying cause is the kLineEdit bug.
When that bug gets fixed, we could do a version check on KF5 libs and #ifdef out this fix. In that case, I think it's clearer to block the signals... but I am happy to change it if that is the preferred solution
in addition, just returning early instead of the assert causes a different bug, which is that compilerEdited gets called when we select out of the compiler, due to the same kLineEdit bug that causes textEdited to be emitted on calling clear().
So, when that happens, compilerEdited deletes the compiler name from the tree view, because in that code path the assert doesn't fire.
I don't have commit access so someone will have to merge this for me. Thanks :-)
Name: Dan Weatherill
email: dan.weatherill@cantab.net