When a project gets reloaded and a folder was added with a cmake option,
folder and file items are created and enqueued in FileManagerListJob.
Then all folder and file items get deleted and when the next
FileManagerListJob::startNextJob() accesses an element in the queue it's
a dangling pointer and a segfault happens.
BUG: 398822
FIXED-IN: 3.0
Create a project like this. Only CMakeLists.txt really matters. The other files just have to be there and have valid code.
CMakeLists.txt like this
cmake_minimum_required(VERSION 3.5)
project(crash)
option(test "build tests" OFF)
add_library(crash
source/lib.cpp
)
if(test)
add_executable(unittest test/test.cpp)
endif(test)source/lib.cpp
int add(int a, int b) {
return a + b;
}test/test.cpp
int main() {
return 0;
}Proceed this steps:
| Lint Skipped |
| Unit Tests Skipped |
Would it make sense to recreate the KDirWatchers at this point? I would prefer if this patch didn't have to get outside the plugin.
Was this question directed to me? If yes, I'm not familiar enough with the code to have a knowledgeable answer.
Hi, sorry it took me such longtime to answer.
I've been trying to reproduce the issue and I've been sadly unable to reproduce it.
Can you still reproduce?
In general, the folders shouldn't need re-creation since they're always there, it's just the target, and the target is not dealt with by the FileManager bit. Am I missing something?
Don't worry, no problem.
I can still reproduce the crash on two machines I have access to. This seems to be a race. Items are added to a queue in FileManagerListJob and deleted afterwards while still beeing in the queue. There is a more thorough description in the bug report.
Btw, I just tested with a release build.
sorry for the long delay, I just tested this and I can reproduce the crash. But just like Aleix, I'm opposed to the suggested way of fixing it. The patch increases coupling without solving the underlying problem. To me, it looks like the list job simply must not store raw pointers, as that's inherently unsafe when we think about changes being done during the queued signal emission... I think to fix this properly, we need to refactor the listjob, and I have an idea on how to do that (operate on indexed strings instead of items, only lookup item when listjob has finished). I'll try this out now and get back to you afterwards.
No problem, I could work with my patch :)
I'm glad you could reproduce the bug. If there is something I can do to help you with the implementation let me know.
sadly my initial attempt doesn't work either ;-) I'm looking at this some more now and will let you know once I've found an alternative approach
finally got it working, man that old code was broken :D I wrote most of that a really long time ago, and it shows!
https://invent.kde.org/kde/kdevelop/merge_requests/4
I'll commandeer this request and close it then, we will continue the discussion on gitlab then.