Bug 419719 - Crash when repeating pgrep <search string> operation back-to-back
Summary: Crash when repeating pgrep <search string> operation back-to-back
Status: RESOLVED FIXED
Alias: None
Product: kate
Classification: Applications
Component: search (show other bugs)
Version: Git
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-06 13:49 UTC by Gernot Gebhard
Modified: 2020-05-06 19:14 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gernot Gebhard 2020-04-06 13:49:50 UTC
SUMMARY

Kate crashes when the user performs a pgrep operation one after another.


STEPS TO REPRODUCE

1. Open a file that resides in a large (git) project
2. Hit F7
3. pgrep muh
4. Hit ESC
5. Repeat 2. and 3. directly thereafter 

Sometimes you have to do the steps 2. - 5. multiple times to observe the crash.


OBSERVED RESULT

Segmentation fault (core dumped)

I'm sorry to not provide a backtrace.
I've compiled kate via (see https://kate-editor.org/build-it/):

cmake .. -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_INSTALL_PREFIX=~/kde/usr \
  -DCMAKE_PREFIX_PATH=~/kde/usr


EXPECTED RESULT

No crash.


SOFTWARE/OS VERSIONS

Linux/KDE Plasma: Manjaro
KDE Plasma Version: 5.8.13
KDE Frameworks Version: 5.68.0
Qt Version: 5.14.1
Comment 1 Christoph Cullmann 2020-04-06 18:53:13 UTC
CC search plugin maintainer:

Kåre, looks like we have some race condition.
Comment 2 Kåre Särs 2020-04-07 06:00:48 UTC
I tried to reproduce but could not....

BTW Christoph, where did you find my old email address? I have not had access to that email since 2004 ;)
Comment 3 Gernot Gebhard 2020-04-07 07:12:04 UTC
(In reply to Kåre Särs from comment #2)
> I tried to reproduce but could not....

I just recompiled kate from scratch just to be sure and could directly reproduce the crash.

To get this behavior, the pgrep operation needs still to be running.
I could do this as described by typing:

  F7
  pgrep muh
  ESC
  F7
  Arrow Up
  Enter

The last four key strokes have to be really quick!
Comment 4 Christoph Cullmann 2020-04-07 07:25:51 UTC
> BTW Christoph, where did you find my old email address? I have not had access to that email since 2004 ;)

:=) My browser proposed that in the bugzilla field ;=)
Comment 5 Kåre Särs 2020-04-09 05:53:41 UTC
I was just about to ask for a crash-dump, but I can actually reproduce...

My crash happens in SearchDiskFiles.cpp while calling QUrl::fromUserInput() in searchSingleLineRegExp()

For some reason fileName is a Null QString, but the QFile taking that as parameter is open....

I'll investigate more...

My first guess is that the file list (m_files) is cleared while we are iterating over it....
Comment 6 Christoph Cullmann 2020-04-26 14:35:44 UTC
Perhaps the public

SearchDiskFiles::startSearch

function shall ensure that no search is still running and cancel any running one by 

m_cancelSearch = true;
wait();

before changing the members and restarting the thread.
Comment 7 Kåre Särs 2020-04-26 19:36:34 UTC
I have now started looking into this and it is a bit complicated.

I have fixed the crash, but I still have some minor annoyances that I want to iron out.

Basically the "thread-safety" was built on the GUI not being enabled, but with the commands we do not have that...
Comment 8 Kåre Särs 2020-05-02 06:28:10 UTC
Git commit 54b730ac6c28d0dddac5e8c583e6ed0b3a0941fb by Kåre Särs.
Committed on 02/05/2020 at 06:26.
Pushed by sars into branch 'Improve--thread-safety-in-search-plugin'.

Fix crash if search command is used while searching

Thread safety was only provided by disabling the UI and that breaks
if we use the commands.

Add terminateSearch functions that stop the search without sending
signals and prevent already queued signals from adding matches in the
matches tree.

M  +11   -1    addons/search/FolderFilesList.cpp
M  +3    -0    addons/search/FolderFilesList.h
M  +17   -3    addons/search/SearchDiskFiles.cpp
M  +2    -0    addons/search/SearchDiskFiles.h
M  +27   -14   addons/search/plugin_search.cpp
M  +10   -8    addons/search/plugin_search.h
M  +11   -0    addons/search/replace_matches.cpp
M  +2    -0    addons/search/replace_matches.h
M  +27   -13   addons/search/search_open_files.cpp
M  +7    -3    addons/search/search_open_files.h

https://invent.kde.org/kde/kate/commit/54b730ac6c28d0dddac5e8c583e6ed0b3a0941fb
Comment 9 Kåre Särs 2020-05-06 19:14:06 UTC
Git commit 69c962f208ce1d608d1f66960e2a2369315ce5ed by Kåre Särs.
Committed on 06/05/2020 at 19:11.
Pushed by sars into branch 'master'.

Fix crash if search command is used while searching

Thread safety was only provided by disabling the UI and that breaks
if we use the commands.

Add terminateSearch functions that stop the search without sending
signals and prevent already queued signals from adding matches in the
matches tree.

M  +11   -1    addons/search/FolderFilesList.cpp
M  +3    -0    addons/search/FolderFilesList.h
M  +17   -3    addons/search/SearchDiskFiles.cpp
M  +2    -0    addons/search/SearchDiskFiles.h
M  +27   -14   addons/search/plugin_search.cpp
M  +10   -8    addons/search/plugin_search.h
M  +11   -0    addons/search/replace_matches.cpp
M  +2    -0    addons/search/replace_matches.h
M  +27   -13   addons/search/search_open_files.cpp
M  +7    -3    addons/search/search_open_files.h

https://invent.kde.org/kde/kate/commit/69c962f208ce1d608d1f66960e2a2369315ce5ed