Created attachment 131414 [details] Backtrace SUMMARY I can't run the current face detection. There is a seg fault. In previous digikam version I scanned everything. So I tried with complete new can (with merge) an skip already scanned. Both end up in a seg fault. By the way it is a big image library with around 20'000 pics. STEPS TO REPRODUCE 1. Just ran the face detection (I did not try with a smaller set) OBSERVED RESULT EXPECTED RESULT SOFTWARE/OS VERSIONS Digikam: Commit 2fdab922ec229c29be1698e9dce59b80dd6cfda6 Linux/KDE Plasma: Kubuntu 20.04 (available in About System) KDE Plasma Version: 5.18.5 KDE Frameworks Version: 5.68.0 Qt Version: 5.12.8 ADDITIONAL INFORMATION
Created attachment 131415 [details] Run without debug output
Created attachment 131416 [details] Run with debug output
Created attachment 131417 [details] Error with skipping already scanned images
The backtrace has already seen somewhere and Maik tried already to fix the problem i think... Gilles
NGhia, Can you reproduce the crash on your computer ? Gilles
Hi Gilles, Yes, I can reproduce it with a huge trunk of images. I am investigating it. I suspect that somewhere in faces pipeline where I changed QImage to QImage* could cause the problem. As I remember, it does not involve face detection, but it might link somewhere. With the backtrace, I think the crash might be during the rendering of cropped faces to Widget. Nghia
Hi Nghia, Do you use also a Ubuntu like Linux ? Gilles
(In reply to caulier.gilles from comment #7) > Hi Nghia, > > Do you use also a Ubuntu like Linux ? > Yes, I use Ubuntu 18.4. > Gilles
Here under Mageia and Centos, i cannot reproduce the crash. As you know i parsed my huge family collection of photo. So, the difference can be something in the kernel about memory allocation or a different ulimit settings. Gilles
(In reply to caulier.gilles from comment #9) How large is your collection? I found that it crashes after found about 3000 faces. Nghia
Around 200 000 photo, with JPEG (mostly), RAW, PNG, and TIFF. I use 32 Gb of RAM, so if there is a memory leak, i cannot see the dysfunction, but i already check the system memory while scanning, and it's very stable. Gilles
I will go through this problem while reform faces management in preparation for the portage to DPlugin. I hope I will fund somethings
I checked my library and there are around 60'000 pics and 3'500 unknown faces and about the same number tagged. The error shows up around 10 to 20 seconds after starting the process. It works on a smaller set of pics. If I can check anything just say it.
The problem is a race condition in the preview loading thread. I've spent a lot of time with it and think I have a good overview of the function. It is programmed very amboned. I have a new idea how the loading notification can be exchanged between the threads safely. I'll be programming a major change for testing. Why the problem mainly occurs under Ubuntu is a mystery to me. Maik
Marcel, a GDB backtrace would be good to see in which function it crashes. https://www.digikam.org/contribute/ Maik
Maik, Can you remember me which Preview engine call is in fault from the Face detection process ? I want mean, if the face engine call to quickly the preview engine and create race conditions, perhaps we can lock the process step ? Creating a unit test to run Preview Engine in multi-thread with plenty of images to render can help to debug this code. Gilles
I think a unit test won't help. The problem is that all processes are brought together via the static loading cache, including the thumbnail threads / tasks. Unfortunately, I and apparently you cannot reproduce the crash. All loading tasks use a non-recursive mutex because a QWaitCondition is used. But this also means that all processes start when the mutex is unlocked. I hope that the problem with calling the wrong overloaded function due to my change to explicit float type in the progress info function is a thing of the past, so far this crash has not been reported again. Maik
Another problematic feature of the preview loader is the loading notification. This is only used by a single function in digiKam. It's the color properties tab. The idea is good, if in the meantime another thread has already loaded the image completely, the current request is canceled and the new image requested. I debugged it. The chance of it entering was zero after 1 hour of intensive use. But thousands of notifications were sent about the addresses stored in the loading cache, if one of these taks no longer exists in the meantime, it would crash. Maik
Git commit bd222b146bc7516378646efd3fff8ac5bf7bb737 by Maik Qualmann. Committed on 04/09/2020 at 19:07. Pushed by mqualmann into branch 'master'. disable loading notification for a test M +1 -1 core/libs/threadimageio/fileio/loadsavetask.cpp M +1 -1 core/libs/threadimageio/preview/previewtask.cpp M +1 -1 core/libs/threadimageio/thumb/thumbnailtask.cpp https://invent.kde.org/graphics/digikam/commit/bd222b146bc7516378646efd3fff8ac5bf7bb737
Marcel, it would be good if you would test the change. Maik
See my face detection session with 7.2.0-beta2 Appimage bundle: https://youtu.be/KnJVSsu0nX8 There is no memory leak here, no crash at all. Best Gilles
Note the HD version of the session is under processing by YouTube. Please wait... Gilles
(In reply to Maik Qualmann from comment #15) > Marcel, a GDB backtrace would be good to see in which function it crashes. > > https://www.digikam.org/contribute/ > > Maik The backtrace was attached from the beginning on. See Backtrace.txt
Created attachment 131427 [details] Memory consumption of the process before it crashs I checked the new commit from Maik. It looks much better. Anyway I still have a seg fault later in time. So it takes longer until it crashes. But now I think it is a memory leak. See the attached picture MemoryConsumption.png
Created attachment 131428 [details] endless loop without crash On another try with gdb the behavior was even more strange. There is no seg fault after running out of memory but digikam is still running but does not check any pictures for faces. You can see this on the network history as all my pics are on a network drive. See MemoryConsumptionSlowDown.png
Marcel, If there is a memory leak somewhere with your Linux box (i see nothing with mine) valgrind can certainly help to identify the dirty code. Look here to see how to run digiKam in valgrind (running speed is decreased by 20). https://www.digikam.org/api/index.html#memleak I full valgrind console trace can be instructive as the memory leak can be located outside digiKam... Gilles
Created attachment 131429 [details] Valgrind output Here is the output of valgrind. As far as I understand this output, there is a problem in dnnfacedetectorssd.cpp.
Hi Marcel, The problem is not directly in dnnfacedetectorssd. Look well the back-trace. All continue in low level inside... OpenCV API. The memory problem is at this place. This is what we suspect since a while: the big puzzle named OpenCV as too many way to optimize computation / process/ data management, using hardware, as GPU. I'm in contact with team from KDenlive project (video montage application) which use also OpenCV in background with the same problem. The solution is to disable optimizations at OpenCV compilation. I do it for all bundles, but it's not possible for native digiKam version where we don't have control of OpenCV configuration. So the ultimate solution is to find a real solution in digiKam code to prevent this memory leak. The problem is to find the context where this dysfunction appear, and it's certainly sue to compilation option in OpenCV, as, i never reproduce this memory leak on all Linux version installed in my computer since many year. This want mean that OpenCV here is compiled without a specific optimization. The Q is which one? If you look in OpenCV CMake top level options, you will see a very huge list of possibility. It's completely crazy... This is what's i use for the bundles : https://invent.kde.org/graphics/digikam/-/blob/master/project/bundles/3rdparty/ext_opencv/CMakeLists.txt#L11 Another possibility, is a problem with a specific kernel option or a GPU driver bug, used indirectly by OpenCV. Here again, the complexity to found the reasons is similar to the distance which separate the sun and the earth. Gilles
Marcel, The video of face detection session is done with the last AppImage bundle computed yesterday with master code. Can you check to see if you reproduce the dysfunction with this version ? https://files.kde.org/digikam/ Gilles
Marcel, Nghia, Another difference between AppImage and the native OpenCV is the version used. AppImage : 3.4.11 LTS Marcel computer : 4.2.0 The last stable OpenCV is 4.4.0. Which OpenCV version we need to use ? With the new DNN code, do we need to switch definitevely to 4.4.0 or i can stay on 3.x LTS ? Gilles
Nghia, Another to take a care in digiKam code is the way to wrap all OpenCV code with C++ exception catcher. OpenCV can prevent a crash if client application is able to capture and exception due to an identified problematic condition in OpenCV API. I already patch few month ago all OpenCV use in face engine, but i don't yet check if your new code repect this rule. Have you take a care about this coding rule ? Gilles
(In reply to caulier.gilles from comment #29) > Marcel, > > The video of face detection session is done with the last AppImage bundle > computed yesterday with master code. Can you check to see if you reproduce > the dysfunction with this version ? > > https://files.kde.org/digikam/ > > Gilles Gilles, I tested it with the AppImage bundle from yesterday. I guess we have at least two errors. The fix from Maik is not included in that bundle. I get again the original bug (actually I got it 2 times of 3). The time it worked, I didn't see any increase in memory. So I guess you are right with the openCv version, but as I mentioned, I can't really test it.
MArcel, You are right, Maik patches are not yet includes in AppImage. I start Appimage 64 build now. Files will be published in few hours. Gilles
(In reply to caulier.gilles from comment #28) > Hi Marcel, > > The problem is not directly in dnnfacedetectorssd. Look well the back-trace. > All continue in low level inside... OpenCV API. The memory problem is at > this place. > > This is what we suspect since a while: the big puzzle named OpenCV as too > many way to optimize computation / process/ data management, using hardware, > as GPU. > > I'm in contact with team from KDenlive project (video montage application) > which use also OpenCV in background with the same problem. > > The solution is to disable optimizations at OpenCV compilation. I do it for > all bundles, but it's not possible for native digiKam version where we don't > have control of OpenCV configuration. > > So the ultimate solution is to find a real solution in digiKam code to > prevent this memory leak. The problem is to find the context where this > dysfunction appear, and it's certainly sue to compilation option in OpenCV, > as, i never reproduce this memory leak on all Linux version installed in my > computer since many year. This want mean that OpenCV here is compiled > without a specific optimization. The Q is which one? > > If you look in OpenCV CMake top level options, you will see a very huge list > of possibility. It's completely crazy... > > This is what's i use for the bundles : > > https://invent.kde.org/graphics/digikam/-/blob/master/project/bundles/ > 3rdparty/ext_opencv/CMakeLists.txt#L11 > > Another possibility, is a problem with a specific kernel option or a GPU > driver bug, used indirectly by OpenCV. Here again, the complexity to found > the reasons is similar to the distance which separate the sun and the earth. > > Gilles Gilles, you are right about the memory leak in openCV. For me it's the first time reading a valgrind output and I want to understand it. From my understanding there are more problems then just the one in openCV. Whats about the one QRect::operator& or QRect::isValid() or qRound(double) or Digikam::DNNFaceDetectorBase::correctBbox. I guess in most examples it's just because of an uninitialized variable, but shouldn't that be fixed anyway. Can't this lead to undefined behavior in same strange cases?(In reply to caulier.gilles from comment #28) > Hi Marcel, > > The problem is not directly in dnnfacedetectorssd. Look well the back-trace. > All continue in low level inside... OpenCV API. The memory problem is at > this place. > > This is what we suspect since a while: the big puzzle named OpenCV as too > many way to optimize computation / process/ data management, using hardware, > as GPU. > > I'm in contact with team from KDenlive project (video montage application) > which use also OpenCV in background with the same problem. > > The solution is to disable optimizations at OpenCV compilation. I do it for > all bundles, but it's not possible for native digiKam version where we don't > have control of OpenCV configuration. > > So the ultimate solution is to find a real solution in digiKam code to > prevent this memory leak. The problem is to find the context where this > dysfunction appear, and it's certainly sue to compilation option in OpenCV, > as, i never reproduce this memory leak on all Linux version installed in my > computer since many year. This want mean that OpenCV here is compiled > without a specific optimization. The Q is which one? > > If you look in OpenCV CMake top level options, you will see a very huge list > of possibility. It's completely crazy... > > This is what's i use for the bundles : > > https://invent.kde.org/graphics/digikam/-/blob/master/project/bundles/ > 3rdparty/ext_opencv/CMakeLists.txt#L11 > > Another possibility, is a problem with a specific kernel option or a GPU > driver bug, used indirectly by OpenCV. Here again, the complexity to found > the reasons is similar to the distance which separate the sun and the earth. > > Gilles Gilles, you are right about the memory leak in openCV. For me it's the first time reading a valgrind output and I want to understand it. From my understanding there are more problems then just the one in openCV. Whats about the one QRect::operator& or QRect::isValid() or qRound(double) or Digikam::DNNFaceDetectorBase::correctBbox. I guess in most examples it's just because of an uninitialized variable, but shouldn't that be fixed anyway. Can't this lead to undefined behavior in same strange cases?
(In reply to caulier.gilles from comment #33) > MArcel, > > You are right, Maik patches are not yet includes in AppImage. > > I start Appimage 64 build now. Files will be published in few hours. > > Gilles Gilles, I tested the new build. For the test I used optimized build (not debug). There it took longer for the error (around an hour) but the memory leak still exists.
So the way to build AppImage with an optimized OpenCV improve the situation but do not solve it. How the memory allocation grow with AppImage version ? less faster than the navive digiKam version ? It can be usefull to compare the backtrace with the debug version of AppImage bundle. run it with "debug" argument to caught crash with GDB. Other Q : which kind of image format do you parse while face detection ? Do you see a memory leak difference with JPEG, PNG, RAW, TIFF for ex ? Gilles
Marcel, Looking your full valgrind trace do not show any memory leak, only uninitialized values. As explain here : https://stackoverflow.com/questions/2612447/pinpointing-conditional-jump-or-move-depends-on-uninitialized-values-valgrin This can be considerate as false positive and in all cases not the real case of the leak of memory from face detection. How to investigate better : 1/ always use a digiKam compiled with debug symbols using valgrind. 2/ The whole memory leak trace is generally reported at end of valgrind session, when digiKam is down. Valgrind will take time to list leak sorted by size. This part of valgrind report is missing in your trace. Gilles
*** Bug 426236 has been marked as a duplicate of this bug. ***
Created attachment 131443 [details] Memory of digikam-7.2.0-beta1-20200905T070522-x86-64-debug.appimage
Created attachment 131444 [details] Memory of digikam-7.2.0-beta1-20200905T070220-x86-64.appimage
I just uploaded the comparison of two appimages (normal and debug). I stopped digikam searching for faces manually before running out of memory because I was not able to take a screen shot after running out of memory. My computer was not reacting any more. That is nasty. Both were taken after a restart of the computer because after the crash the swap is still in use. So I got a cleaner record Personally I can't see any significant difference. Why and when should I take a backtrace? Where can provide the debug argument? There is no such argument on appimages. I have the following pics in my lib: BMP: 13 GIF: 4 JPEG: 1 JPG: 59875 PNG: 758 PSD: 2 TIFF: 1 So I guess the leak must come at least with JPG or png pictures.
About the Valgrind trace, all unititialized memory entries are like this : ==11721== at 0x56FDAA1: int const& qMin<int>(int const&, int const&) (qglobal.h:594) ==11721== by 0x5EB28CE: Digikam::DNNFaceDetectorBase::correctBbox(cv::Rect_<int>&, cv::Size_<int> const&) const (dnnfacedetectorbase.cpp:127) ==11721== by 0x5EAE704: Digikam::DNNFaceDetectorSSD::postprocess(cv::Mat, cv::Size_<int> const&, std::vector<cv::Rect_<int>, std::allocator<cv::Rect_<int> > >&) const (dnnfacedetectorssd.cpp:144) ==11721== by 0x5EAE18F: Digikam::DNNFaceDetectorSSD::detectFaces(cv::Mat const&, cv::Size_<int> const&, std::vector<cv::Rect_<int>, std::allocator<cv::Rect_<int> > >&) (dnnfacedetectorssd.cpp:78) ==11721== by 0x5EAD201: Digikam::OpenCVDNNFaceDetector::cvDetectFaces(cv::Mat const&, cv::Size_<int> const&) (opencvdnnfacedetector.cpp:274) ==11721== by 0x5EAD09C: Digikam::OpenCVDNNFaceDetector::detectFaces(cv::Mat const&, cv::Size_<int> const&) (opencvdnnfacedetector.cpp:245) ==11721== by 0x5E8C7D5: Digikam::FaceDetector::detectFaces(Digikam::DImg const&, QSize const&) (facedetector.cpp:289) ==11721== by 0x4E2E481: Digikam::DetectionWorker::process(QExplicitlySharedDataPointer<Digikam::FacePipelineExtendedPackage>) (detectionworker.cpp:55) ==11721== by 0x4E0CD4B: Digikam::DetectionWorker::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_detectionworker.cpp:87) All are relevant of a use of qMax/qMin from Qt in code using OpenCV API. I will use cv::max/cv::min instead, just to be homogeneous, but it's not the source of the real memory leak problem. Gilles
Marcel, There is another possible test to do : use a VirtualBox instance, running the same version of Ubuntu, and check if memory leak appear. As VirtualBox use an emulated basic video card, and as OpenCV try to use optimisations from GPU, is there is a link between video driver and the memory leak, we will see imediatly. Q: which real video card do you use in your computer ? Gilles
(In reply to caulier.gilles from comment #43) > Marcel, > > There is another possible test to do : use a VirtualBox instance, running > the same version of Ubuntu, and check if memory leak appear. > > As VirtualBox use an emulated basic video card, and as OpenCV try to use > optimisations from GPU, is there is a link between video driver and the > memory leak, we will see imediatly. > > Q: which real video card do you use in your computer ? > > Gilles I'm checking that with virtualbox. I'm using a Intel Corporation HD Graphics 620 (rev 02)
Marcel, from your comment #41 : >I just uploaded the comparison of two appimages (normal and debug). I stopped >digikam searching for faces manually before running out of memory because I was >not able to take a screen shot after running out of memory. My computer was not >reacting any more. That is nasty. Both were taken after a restart of the >computer because after the crash the swap is still in use. So I got a cleaner >record Your system do not have enough free meory to run properly. It's normal. You reach the limits. >Personally I can't see any significant difference. As expected. >Why and when should I take a backtrace? The backtrace with GDb can be done with Appimage to pass "debug" as argument in command line when you start the AppImage bundle. This will run digiKam in GDB. When it crash, enter "bt" in GDB shell. >Where can provide the debug argument? There is no such argument on appimages. In the bundle, there is a bash script to catch arguments. I written the script as AppImage SDK do not provide this kind of feature, even if i talk with AppImage lead developer, but he won't to support this as i know. Note : just for info : the official AppImage target builder tool from the SDK is unable to compile the bundle. I reported this problem to the AppImage project, which responded that the target application is to huge to be processed. Seriouly ! AppImage has used bash scripts in the pass and all work as expected. They convert all bash script to C program, and now nothing work: the program crash. As usual, developer use this kind of excuse to not fix the real problem... Read the story here: https://github.com/probonopd/linuxdeployqt/issues/359 This is why all the digiKam AppImage build process run through bash scripts that i written. There is no stupid limit. >I have the following pics in my lib: >BMP: 13 >GIF: 4 >JPEG: 1 >JPG: 59875 >PNG: 758 >PSD: 2 >TIFF: 1 >So I guess the leak must come at least with JPG or png pictures. JPG probably, i think. It can be also a problem with libjpeg-turbo which try to optimize also the JPEG processing using GPU. If it true, decoding/encoding JPEG images with BQM for ex can be a try to check if the problem exists. Gilles
Git commit a98a9d0a5a8a6e69ae60ed8045d1bf6e885fc84a by Gilles Caulier. Committed on 06/09/2020 at 08:23. Pushed by cgilles into branch 'master'. qMin => cv::min / qMax => cv::max M +18 -14 core/libs/facesengine/detection/opencv-dnn/dnnfacedetectorbase.cpp https://invent.kde.org/graphics/digikam/commit/a98a9d0a5a8a6e69ae60ed8045d1bf6e885fc84a
Marcel, As you are developer (:=))... I can give you some pointer to AppImage build script used by digiKam. https://invent.kde.org/graphics/digikam/-/tree/master/project/bundles/appimage I written all of them, since many years now. So i learn in-deep core AppImage to understand all the functionalities, the limits, the capabilities, etc. The lead README explain all the stuff. Just read an ask question. One note: the 01 script require Mageia6 system. I use this Linux box because i know very well, but another ones can be used. The package names just need to be changed. In all case, never use Ubuntu for that. This Linux is just a pain and a waste of time (to develop). Centos, Suse, Debian, etc, are fine. The startup up scrip used internally in the bundle to launch digiKam is this one : https://invent.kde.org/graphics/digikam/-/blob/master/project/bundles/appimage/data/AppRun Voilà. Gilles
Created attachment 131445 [details] Seg fault in virtual Box running Kubuntu I started digikam in a virtualBox running Kubuntu. It was hard to start digikam with a total of 8 GB of RAM. Digikam run not before a total of 6 GB of my memory was given. So first the good news. During the scan (approximately 1 minutes). The memory did not noticeable increase. Digikam crashed with the seg fault in segfault_virtualBox.txt. I hope I can give with this valuable information. How can I switch my JPEG decoding/encoding to BQM that is not using my GPU?
>I started digikam in a virtualBox running Kubuntu. It was hard to start digikam >with a total of 8 GB of RAM. Digikam run not before a total of 6 GB of my memory >was given. 6Gb at minimum to start ? Impossible. I run digiKam with 3Gb computer (laptop), and it work. It's definitively a Ubuntu problem, i'm sure... This is why Ubuntu is a weird, dirty, wrong system at all... >So first the good news. During the scan (approximately 1 minutes). The memory >did not noticeable increase. Digikam crashed with the seg fault in >segfault_virtualBox.txt. I hope I can give with this valuable information. Well, 6Gb or ram is definitively abnormal. Don't forget that we compile digiKam for 32 bits target, where only 4Gb is available, and it work... >How can I switch my JPEG decoding/encoding to BQM that is not using my GPU? You cannot. libjpeg-turbo is compiled with optimization as well. Just create a BQM queue, fill with few JPEG, in the workflow convert JPEG to another JPEG, and look the memory allocation. Another Q: try showfoto and look if memory allocation is also huge. AppImage provide also showfoto. Just pass "showfoto" as argument to the command line. With digiKam there is also a possibility that database interface leak memory while operating. This will operate at startup and while scanning faces. In both case, database commits are done. As Showfoto do not use a database, this will be an indicator. Gilles
Marcel, I installed a Kubuntu 20.04 under VirtualBox with 8Gb of Ram. This is the memory allocation while startup with Appimage 7.2.0-beta1. There is just a single SQlite managed collection with no images. So scan do nothing. https://i.imgur.com/V0iocMJ.png Gilles
Marcel, I forget to said that memory allocation from comment #50 and done under Kubuntu is measured with AppImage bundle including debug symbols. So binary objects in memory are really more important than without debug symbols, of course. Gilles Caulier
Arf. It's clear. The problem is not located in face scan under Kubuntu. It's a memory leak done by the system, because it do not release memory at all. When i close my digiKam session, ksysguard still showing the memory allocated as well. If i restart to another digiKam session, the system continue to allocate over and other and it finish to dead, OF COURSE. Definitively, Ubuntu done a worse memory management in the background. How a system like can exists, seriously ! Gilles Caulier
I tried to run Showfoto from the AppImage. It start quickly and memory allocation is ridiculous, and resumed well at end of Showfoto session. So this problem of memory leak is : 1/ not relevant at all of faces scanning. 2/ not releavant of plugins loaded in memory. Showfoto use same plugins list (excepted ones from BQM), and there is no side effect in memory. 3/ probably relevant of database init and use as it's the main difference with Showfoto. Gilles
Forget my last comment #53. Wth AppImage 7.2.0-beta1, I compared the memory allocation between Kubuntu and Mageia7, and the behaviour is exactly the same : 4 Gb used by digiKam at startup. With or without debug symbols, it's the same (here i don't understand why). And at end of digiKam session, both system the memory is free. ouf... So yes, the memory leak is well located in Face detection... Note : i disabled all plugin (generic and editor), and restarted digiKam. This is have no effect on memory allocation : always 4 Gb when digiKam has completed the startup stage. Gilles
Marcel, Kubuntu 20.04 installed and ready to compile digiKam from git/master, of course in a VM. Kubuntu package is really a puzzle and lots of inter-dependencies are not resolved automatically or set by default. It's really a pain... Q: did you enable the 3D acceleration in VM video settings ? Gilles
(In reply to caulier.gilles from comment #55) > Marcel, > > Kubuntu 20.04 installed and ready to compile digiKam from git/master, of > course in a VM. Kubuntu package is really a puzzle and lots of > inter-dependencies are not resolved automatically or set by default. It's > really a pain... > > Q: did you enable the 3D acceleration in VM video settings ? > > Gilles Of course as we are looking for a memory leaks I tried to disable all hw accelerations. Glade you experience the same as I do with the memory demand. So does Digikam still run on your 3GB RAM Machine? If you want I have an ansible playbook to install all dependencies for compiling digikam (or just read it from there)
digiKam is compiled and run on my Kubuntu VM. I playing with valgrind. 3D acceleration is disabled. I imported 20 Gb of family photo. I will start a scan faces and see. Note : typically, digiKam must start on a 32 bits system so with less than 4Gb or RAM. I suspect that huge memory allocation at startup is due the problem listed on file #426185. Gilles
Marcel, Maik, The native compiled version of digiKam from git/master running on my Kubuntu 20.04 VM run since more than 1 hour in valgrind. it scan for new items. After the startup stage and a memory allocated up to 6.4 Gb (all the system). It's stable since a while. Scanning for new items do not leak memory. All digiKAm settings are all defaults. Database is a sqlite one hosted in ~/Pictures directory. I uploaded 20 Gb or photos from my lead computer for testing, all containing more and less 1 face. digikam version 7.2.0-beta1 Images: JPG: 1108 PGF: 1 PNG: 16 RAW-ARW: 143 RAW-RAF: 9 total: 1277 : Videos: MOV: 1 total: 1 : Total Items: 1278 Albums: 36 Tags: 37 : Database backend: QSQLITE Database Path: /home/gilles/Pictures/ !thz sc
MArcel, Maik, The scan for new items is now complete. I will close the valgrind session to see if memory leak are reported by the analyzer. Gilles Caulier
The cause of the high memory consumption at the beginning in digiKam-7.2.0 is bug 426185. We load the file "/usr/share/digikam/facesengine/openface_nn4.small2.v1.t7" into memory 40 times. Maik
Created attachment 131448 [details] valgrind session Kubuntu 20.04 7.2.0-beta1 scan new items
Maik, yes, this what i have suspected. I start now the valgrind session with face scan... Please wait. Gilles
(In reply to Maik Qualmann from comment #60) > The cause of the high memory consumption at the beginning in digiKam-7.2.0 > is bug 426185. We load the file > "/usr/share/digikam/facesengine/openface_nn4.small2.v1.t7" into memory 40 > times. > > Maik Hi Maik, I find that we create 3 different FacialRecognitionWrapper instances in FaceManement. Can I change this class into singleton to assure the coherence of faces engine and avoid memory consumtion? Nghia
Created attachment 131449 [details] heaptrack.jpg It even has to be FacialRecognitionWrapper 4 times, because I see the 10 block of the debug output 4 times. Here is an analysis from the start with Heaptrack. Maik
Yes, it has to be 4 times but I found only 3 instances in FacesManagement. But either way, the solution to this problem can be making this class a singleton since it is nothing but an interface to the recognition of faces engine. Nghia
The main problem is the "shapepredictor.dat", this file is also loaded 40 times into the memory. Maik
Maik, Nghia proposal from coment #63 is the right to go i think. At least if data model files must be loaded and shared in memory by read only, a singleton with public accessors can be optimum the solution. Gilles
Created attachment 131452 [details] valgrind session Kubuntu 20.04 7.2.0-beta1 scan faces This valgrind trace start when scan face process is launch an stop at end of digiKam session. The huge memory leak is not really visible here.
(In reply to Maik Qualmann from comment #66) > The main problem is the "shapepredictor.dat", this file is also loaded 40 > times into the memory. > > Maik shapepredictor.dat is loaded with FaceExtractor. Therefore, when we avoid the duplicate of this object, we can reduce the memory consumption. For now, I used 10 FaceExtractor objects to x3 speed of facial recognition. If you like I can reverse it back to use only 1 object. Nghia
This is a desktop video capture under Kubuntu 20.04 VM session running compiled digiKam 7.2.0-beta1 while face detection inside valgrind. Look well the memory allocation with ksysguard. Yes, we allocate memory and clear step by step. There is not cumulative memory leak visible. Of course, using valgrind, this decrease running speed by 20 but the behaviour is similar than under Mageia7 tested yesterday. Nghia, there is no active clustering while faces detection. Right ? If yes, well it's coherent to see no memory allocated to group similar faces before recognition processing. If the Marcel dysfunction exists, how to reproduce exactly ? Outside the fact of cumulative model data files loaded at startup that will reduce the fingerprint at when it will be fixed, i cannot see a memory dysfunction at face scan. Gilles
I miss the video link (:=))) https://youtu.be/iK3Ueh1TAng
(In reply to caulier.gilles from comment #70) > This is a desktop video capture under Kubuntu 20.04 VM session running > compiled digiKam 7.2.0-beta1 while face detection inside valgrind. Look well > the memory allocation with ksysguard. Yes, we allocate memory and clear step > by step. There is not cumulative memory leak visible. > > Of course, using valgrind, this decrease running speed by 20 but the > behaviour is similar than under Mageia7 tested yesterday. > > Nghia, there is no active clustering while faces detection. Right ? > If yes, well it's coherent to see no memory allocated to group similar faces > before recognition processing. > > If the Marcel dysfunction exists, how to reproduce exactly ? Outside the > fact of cumulative model data files loaded at startup that will reduce the > fingerprint at when it will be fixed, i cannot see a memory dysfunction at > face scan. > > Gilles They aren't active but the instances are pre-created along with Workers of FaceManagement.
https://youtu.be/87gFl0VJVRA desktop video recording running digiKam 7.2.0-beta1 without valgrind this time and using single core to faces scan. There is no memory leak at all. Gilles
https://youtu.be/zaiWDqSIyUQ desktop video recording running digiKam 7.2.0-beta1 without valgrind and and using multi-core to faces scan. There is no memory leak at all. Gilles
https://youtu.be/rR0V_CrIRsQ desktop video recording running digiKam 7.2.0-beta1 to faces scan: + without valgrind + using multi-core + 3D acceleration from Virtual Box This time a small memory leak is visible. This will not crash the system as it increase very slowly, and i don't have enough items to scan to overload memory. In all cases, OpenCV + GPU stuff introduce a dysfunction. Note that OpenCV come from the system I don't recommpile myself the library with usual optimization include in AppImage bundle. Other point : It's an emultated 3D acceleration. What's about a real 3D card + proprietary driver, as NVidia or AMD stuff... Gilles Gilles
After all, I found this entry : https://github.com/opencv/opencv/issues/17154 Gilles
And another one : https://github.com/opencv/opencv/issues/10101 Gilles
OpenCV provide a solution to disable 3D aceleration by API call: https://answers.opencv.org/question/123097/how-to-stop-using-gpu-in-opencv-32/?comment=123114 So typically, for each thread using OpenCV API, we need to call cv::ocl::setUseOpenCL(false) But it's problematic to use : https://github.com/opencv/opencv/issues/8035 In all case, we must not use cv::Umat which is optimized for OpenCV and use cv::Mat instead everywhere. Gilles
So, I think it's time to summarize all findings: 1. Disable loading notification This solved the original problem 2. Huge memory demand at startup For this problem is another bug (https://bugs.kde.org/show_bug.cgi?id=426185). Some memory can be reduced anyway bug some is used to gain a speed advantage in digikam. The Question is what is more important. Speed or memory consumption. Personally, this memory for speed gain should definitely not be allocated at startup time as most digikam usages will not scan for faces each time they use digikam. It should be allocated at the start of face detection. 3. Memory leak on face detection There is a bug in openCV with the use of GPU causing that leak. More memory leaks on bigger picture collections. To solve that problem cv::Mat can be used instead of cv::UMat, but again that will decrease the scanning speed. Probably a checkbox "use GPU (experimental)" could help.
I implement singleton on FacialRecognitionWrapper so that it initiated only once. I also reduce the number of FaceExtractor to 1, which is the same as the previous version of faces engine. However, this error still occurs, and I notice that it only occurs when the Unknown face tab is opened and the cropped faces are rendered.
To Marcel from Comment #72: By chance, we do not use c::Umat in digiKam opencv source code. Only cv:Mat is used. So the memory leak can be only located internally in OpenCV libraries. Gilles
Hi all, Reading the OpenCV document, that i try to understand, we have this envirronement variable which can be changed at run time: OPENCV_OPENCL_DEVICE ... as explained here: https://docs.opencv.org/2.4/modules/ocl/doc/introduction.html ... and if i'm not too wrong, if i set export OPENCV_OPENCL_DEVICE=CPU this will only use CPU and never GPU optimization internally with OpenCV. I'm right ? Gilles
(In reply to caulier.gilles from comment #82) > Hi all, > > Reading the OpenCV document, that i try to understand, we have this > envirronement variable which can be changed at run time: > > OPENCV_OPENCL_DEVICE > > ... as explained here: > > https://docs.opencv.org/2.4/modules/ocl/doc/introduction.html > > ... and if i'm not too wrong, if i set > > export OPENCV_OPENCL_DEVICE=CPU > > this will only use CPU and never GPU optimization internally with OpenCV. > > I'm right ? > > Gilles In my knowledge, currently, OpenCV DNN is using CPU as default. For GPU, there is only NVIDIA CUDA is supported and it's required further configuration. So for now in digikam faces engine, all has been processed on CPU.
I respond to myself: This is the right syntax: export OPENCV_OPENCL_DEVICE=:CPU: See comment from this OpenCV issue: https://github.com/opencv/opencv/issues/8275 Gilles
Look in main.cpp, we already have a system setting option for it and is set by default. Further environment variables can be added. Maik
Nghia, well, this is not what's a read. depending of OpenCV version, especially the 4.x, opencv try to use GPU optimizations everywhere if possible. Perhaps DNN code as well is not optimized, but i'm sure that DNN code call internal OpenCV API which are optimized, for ex about matrix management. My test under Kubuntu VM customized with and without 3D acceleration setting demonstrate the dysfunction about memory allocation is lost at run time. Gilles
Right Maik, I forget this part. https://invent.kde.org/graphics/digikam/-/blob/master/core/app/main/main.cpp#L123 ... but the export is set to: OPENCV_OPENCL_DEVICE=null instead of: OPENCV_OPENCL_DEVICE=:CPU: ...as explained in the OpenCV documentation to disable GPU usage. I'm wrong ? Gilles
(In reply to caulier.gilles from comment #86) > Nghia, > > well, this is not what's a read. depending of OpenCV version, especially the > 4.x, opencv try to use GPU optimizations everywhere if possible. > > Perhaps DNN code as well is not optimized, but i'm sure that DNN code call > internal OpenCV API which are optimized, for ex about matrix management. > > My test under Kubuntu VM customized with and without 3D acceleration setting > demonstrate the dysfunction about memory allocation is lost at run time. > > Gilles OpenCV provides GPU optimization for matrix operations but I'm sure that we can actually choose the processing unit for OpenCV::dnn::net via their API. And currently, the only GPU driver supported for OpenCV's neural network is NVIDIA CUDA. By default, all the internal backend of OpenCV::dnn::net are handled by CPU
Gilles, that's a good question. I hadn't read ":CPU:" anywhere so far, rather "null". At the end of the thread someone writes that he was successful with "disable"...OpenCV is a puzzle... Maik
I will said more: it's a big puzzle... To be sure, this evening is will try again with the VM Kubuntu, to set the environment variable to see if there is a difference with the memory allocation. Gilles
Nghia, It still a pending Q in my mind. Currently, all digiKAm bundle still compiled with OpenCV LTS version 3.4.11. There is an advantage to switch definitively to OpenCV 4.x ? Gilles
(In reply to caulier.gilles from comment #91) > Nghia, > > It still a pending Q in my mind. Currently, all digiKAm bundle still > compiled with OpenCV LTS version 3.4.11. > > There is an advantage to switch definitively to OpenCV 4.x ? > > Gilles I doubt that. I saw that OpenCV 4.x has added recent trending models, but most of the current code base is nearly the same. I already tried it out when I tried YOLOv4 on digiKam since YOLOv4 required OpenCV 4.x. The performance is not much different, in my opinion.
Git commit 5102e32b4e9e308df760a45624832caa204d82c0 by Maik Qualmann. Committed on 07/09/2020 at 18:02. Pushed by mqualmann into branch 'master'. enable loading notification again and change stop function M +1 -11 core/libs/threadimageio/fileio/loadingcache.cpp M +16 -19 core/libs/threadimageio/fileio/loadsavetask.cpp M +2 -2 core/libs/threadimageio/preview/previewtask.cpp M +2 -2 core/libs/threadimageio/thumb/thumbnailtask.cpp https://invent.kde.org/graphics/digikam/commit/5102e32b4e9e308df760a45624832caa204d82c0
To Maik comment #89: Reading OpenCV source code give the solution: https://github.com/opencv/opencv/blob/master/modules/core/src/ocl.cpp#L2068 OPENCV_OPENCL_DEVICE=null has no effect. OPENCV_OPENCL_DEVICE=disable must work. OPENCV_OPENCL_DEVICE=:CPU: must work (see the parser https://github.com/opencv/opencv/blob/master/modules/core/src/ocl.cpp#L2034) Gilles
Maik, Setting the 2nd environment variable OPENCV_OPENCL_RUNTIME=disabled is fine: https://github.com/opencv/opencv/blob/master/modules/core/src/ocl.cpp#L1151 For this one it's really "disabled" where the other one is "disable". Take a care to the typo with ending by 'd' or not. This is clearly an error prone syntax... Gilles
Git commit b5e3c5987a7380fdc5ce9a59ee5e62e6acbaf51b by Gilles Caulier. Committed on 08/09/2020 at 03:11. Pushed by cgilles into branch 'master'. Fix typo with OpenCV environnement variable value used to disable OpenCL device for 3D accelerations Related: bug 423632 M +2 -1 core/app/main/main.cpp https://invent.kde.org/graphics/digikam/commit/b5e3c5987a7380fdc5ce9a59ee5e62e6acbaf51b
Marcel, See my comment: https://bugs.kde.org/show_bug.cgi?id=426185#c4 The huge memory allocation at startup is now fixed by Nghia commit: https://invent.kde.org/graphics/digikam/-/commit/28027b21d9fa4368eb2f2ad26dbb1cb33e274cce It still to check if the OpenCL memory leak from OpenCV is fixed with my commit: https://invent.kde.org/graphics/digikam/commit/b5e3c5987a7380fdc5ce9a59ee5e62e6acbaf51b And if the another memory glitch is fixed by Maik commit: https://invent.kde.org/graphics/digikam/commit/5102e32b4e9e308df760a45624832caa204d82c0 I will rebuild AppImage 64 bits this morning. Gilles
(In reply to caulier.gilles from comment #97) > Marcel, > > See my comment: > > https://bugs.kde.org/show_bug.cgi?id=426185#c4 > > The huge memory allocation at startup is now fixed by Nghia commit: > > https://invent.kde.org/graphics/digikam/-/commit/ > 28027b21d9fa4368eb2f2ad26dbb1cb33e274cce > > It still to check if the OpenCL memory leak from OpenCV is fixed with my > commit: > > https://invent.kde.org/graphics/digikam/commit/ > b5e3c5987a7380fdc5ce9a59ee5e62e6acbaf51b > > And if the another memory glitch is fixed by Maik commit: > > https://invent.kde.org/graphics/digikam/commit/ > 5102e32b4e9e308df760a45624832caa204d82c0 > > I will rebuild AppImage 64 bits this morning. > > Gilles Hi Gilles, Even with this version, the crash during face detection still occurs on my machine Nghia
@Nghia, do you have a backtrace? Maik
Git commit f813d64af40c5ea342ab9da340f12acc4b19023d by Gilles Caulier. Committed on 08/09/2020 at 07:23. Pushed by cgilles into branch 'master'. I needs new glasses : disable => disabled Related: bug 423632 M +1 -1 core/app/main/main.cpp https://invent.kde.org/graphics/digikam/commit/f813d64af40c5ea342ab9da340f12acc4b19023d
Git commit cde3403938b82e0cfcccb1b557b6c2319ac8557e by Maik Qualmann. Committed on 08/09/2020 at 10:37. Pushed by mqualmann into branch 'master'. add base classes initialization explicit in the constructor Related: bug 423632, bug 425723 M +2 -0 core/libs/threadimageio/fileio/loadsavetask.cpp M +2 -0 core/libs/threadimageio/fileio/loadsavetask.h https://invent.kde.org/graphics/digikam/commit/cde3403938b82e0cfcccb1b557b6c2319ac8557e
Git commit a06b3c5dcc32a2f95c5fbf0ac2fa898931524cea by Maik Qualmann. Committed on 08/09/2020 at 20:05. Pushed by mqualmann into branch 'master'. add static cast for loading notifikation Related: bug 423632, bug 425723 M +2 -1 core/libs/threadimageio/fileio/loadingcache.cpp https://invent.kde.org/graphics/digikam/commit/a06b3c5dcc32a2f95c5fbf0ac2fa898931524cea
Created attachment 131496 [details] BackTrace after fixing FaceExtractor memory consumtion Hi Maik, Here the backtrace. It's still the same error. Nghia
This backreace is not from the current head of master. Please try the current master version. Maik
(In reply to Maik Qualmann from comment #104) > This backreace is not from the current head of master. Please try the > current master version. > > Maik This is the new back trace : Thread 86 "Thread (pooled)" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fff0bfff700 (LWP 16177)] 0x00007ffff7077a32 in Digikam::PreviewLoadingTask::execute (this=0x55558f9f3240) at /home/minhnghiaduong/Documents/Projects/digikam/core/libs/threadimageio/preview/previewtask.cpp:443 443 m_thread->taskHasFinished(); (gdb) bt #0 0x00007ffff7077a32 in Digikam::PreviewLoadingTask::execute() (this=0x55558f9f3240) at /home/minhnghiaduong/Documents/Projects/digikam/core/libs/threadimageio/preview/previewtask.cpp:443 #1 0x00007ffff708a0f5 in Digikam::LoadSaveThread::run() (this=0x5555606f79a0) at /home/minhnghiaduong/Documents/Projects/digikam/core/libs/threadimageio/fileio/loadsavethread.cpp:134 #2 0x00007ffff70b2fe8 in Digikam::DynamicThread::Private::run() (this=0x55557aff5ba0) at /home/minhnghiaduong/Documents/Projects/digikam/core/libs/threads/dynamicthread.cpp:191 #3 0x00007ffff3eaf2b2 in () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #4 0x00007ffff3eb217d in () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #5 0x00007ffff2cab6db in start_thread (arg=0x7fff0bfff700) at pthread_create.c:463 #6 0x00007ffff3595a3f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 Nghia
Maik, Nghia, The crash under previewtask due to a call of pure virtual method from parent class can by due to a call of pure method in constructor. This is the only case that i know. It explained here: https://stackoverflow.com/questions/99552/where-do-pure-virtual-function-call-crashes-come-from I don't check preview code to see if something like this is code somewhere... Gilles
Another point is the time life of the object where gcc call pure virtual method at run time. It's a race condition in destructor. It's explained here : https://tombarta.wordpress.com/2008/07/10/gcc-pure-virtual-method-called/ Gilles
Hi all, I just tested again this morning under Kubuntu if memory leak while scanning faces is fixed, and i can confirm. Look the desktop video : https://youtu.be/VtRgENoJeNA Here i re-scan and merge results in database, as all image was previously scanned from collection. I set KSysguard to only show the application memory allocation from user space with a large graph. You cannot see a cumulative memory hole. Gilles
Maik, Why under a VM running Kubuntu, i cannot reproduce the crash from preview task? Gilles
Yes, pure virtual functions are not allowed to called in the constructor because they have not yet been created, but that is not the case here in the loader task. The other thing about the deconstructor is interesting. But I suspect that a deleted task remains in the QMap due to a race condition...due to non-recursive mutex... Maik
Yes, that is the question, why can some people reproduce it after just a few seconds and we cannot reproduce it at all. Assumption: Hardware dependent, processor / cache / memory / mainboard? Or the host Linux system? Maik
I have a theory: Look my desktop video. There is no tab open in right sidebar. Perhaps crash from users is due to an extra operation done in right sidebar while processing face detection. Look well the icon view. It's constantly refresh when a new face is found, if you take the focus on the unknown virtual album. This want mean the first item in icon view is never the same when new items are appended. So, the right sidebar is updated accordingly, and dependeing of the active tab, processing in background is started (for ex, in colors view, the image is loaded to compute histogram. This is an example. We can imagine the end users opening image editor to start to fix images while long faces detection, or to compute panorama, or to export images on web services. This can have side effects. In my case, when i test faces management, i lets the job alone. It's long, i will do a dog walk and go back later to check... Gilles
Just thinking loud... I don't think the theorie of gilles is right that there are other tasks runnung. Of course this may lead the other problems but not to the one we are looking for. When I look to the video of the scan I think the main difference is the precondition. I have a library with tousends of taged people. Some with a real tag others just in the db as unknown So in my case the new engine must do a huge merge process. Probably there is the error we are looking for.
Marcel, The Preview Task code where Maik try to fix the crash since a very long time is not relevant of the database access and the merge operation between tags. And we talk about face detection, not face recognition, so the face area identification and ready to use for face tagging (manual or automatic). The Preview Task is only used to get the image contents and pass it to the face engine for analysis. The result is a list or area where faces are located and stored in main database. So my theory still valid, but you are right about face recognition and possible side effects with the rest of the program. It's another point to check later. Gilles
And my theory is that we have a mutex problem. I have a new idea and I will implement it tonight. Maik
Git commit 901227fa96db807e02b71a84c933d34b97ce3ec3 by Maik Qualmann. Committed on 09/09/2020 at 19:56. Pushed by mqualmann into branch 'master'. changes to setStatus() function Related: bug 423632, bug 425723 M +1 -1 core/libs/threadimageio/fileio/loadingcache.cpp M +19 -12 core/libs/threadimageio/fileio/loadsavetask.cpp M +1 -2 core/libs/threadimageio/thumb/thumbnailtask.cpp https://invent.kde.org/graphics/digikam/commit/901227fa96db807e02b71a84c933d34b97ce3ec3
MAik, i append a rules to cppcheck this morning to pass all includes pathes from digiKam/core while analysis. This add more than 700 entries in the reports. https://www.digikam.org/reports/cppcheck/master/ A lots of entries can be dropped as it's relevant of code that we don't maintain (dng sdk, libraw, libpgf, etc.). I will try to fix the analysis to remove these entries automatically. I don't yet check all entries, but perhaps you will found interesting items, as virtual calls in constructors, null pointer deference, missing override, missing explicit constructor, etc. Gilles
Hi all. I'm back from holiday. I tried the process again with the latest version of Digikam (aa4f7ce6522c1c9559619c2dd3af5207a8a451ac). About the huge memory demand: Great job, this is solved. About the memory leak: Hard to say, I think it still leaks a bit (not as much as before). Anyway, for the moment I have enough memory that it does not crash at least because of this leak. About the seg fault: Still the same. Sometimes after a few minutes and sometimes after an hour or more. Here is the most resent backtrace: Thread 54 "Thread (pooled)" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fff81ffb700 (LWP 22073)] 0x0000000000000000 in ?? () (gdb) bt #0 0x0000000000000000 in ?? () #1 0x00007ffff6af3896 in Digikam::PreviewLoadingTask::execute (this=0x555560845610) at /home/marcel/Repos/digikam/core/libs/threadimageio/preview/previewtask.cpp:362 #2 0x00007ffff6b06f6e in Digikam::LoadSaveThread::run (this=0x7fffd400bf80) at /home/marcel/Repos/digikam/core/libs/threadimageio/fileio/loadsavethread.cpp:134 #3 0x00007ffff6b305fa in Digikam::DynamicThread::Private::run (this=0x555557b3b590) at /home/marcel/Repos/digikam/core/libs/threads/dynamicthread.cpp:191 #4 0x00007ffff5301f82 in ?? () from /lib/x86_64-linux-gnu/libQt5Core.so.5 #5 0x00007ffff52fe9d2 in ?? () from /lib/x86_64-linux-gnu/libQt5Core.so.5 #6 0x00007ffff37f8609 in start_thread (arg=<optimized out>) at pthread_create.c:477 #7 0x00007ffff4f72103 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 (gdb) Is there anything else I could test that helps finding this issue?
Git commit 2601c0a8adbd51a3828353e24f5bc169260eb850 by Gilles Caulier. Committed on 19/09/2020 at 10:27. Pushed by cgilles into branch 'master'. Use heap to create DRawInfo instance to prevent stack overflow Related: bug 426597 M +3 -3 core/libs/threadimageio/preview/previewtask.cpp https://invent.kde.org/graphics/digikam/commit/2601c0a8adbd51a3828353e24f5bc169260eb850
Git commit 2ed30b67282e725671e65f5499c6293dc0de7070 by Gilles Caulier. Committed on 19/09/2020 at 10:50. Pushed by cgilles into branch 'master'. Use heap to create DRawInfo instance to prevent stack overflow Related: bug 426597 M +20 -18 core/dplugins/generic/tools/htmlgallery/generator/galleryelementfunctor.cpp https://invent.kde.org/graphics/digikam/commit/2ed30b67282e725671e65f5499c6293dc0de7070
Git commit edf82fbaf85e4131b3e39e4f787e9820a1e273ee by Gilles Caulier. Committed on 19/09/2020 at 10:52. Pushed by cgilles into branch 'master'. Use heap to create DRawInfo instance to prevent stack overflow Related: bug 426597 M +5 -5 core/dplugins/dimg/raw/dimgrawloader.cpp https://invent.kde.org/graphics/digikam/commit/edf82fbaf85e4131b3e39e4f787e9820a1e273ee
Git commit f7521ff595caaee5a85fdd75354ac78a753e39dd by Gilles Caulier. Committed on 19/09/2020 at 10:54. Pushed by cgilles into branch 'master'. Use heap to create DRawInfo instance to prevent stack overflow Related: bug 426597 M +80 -80 core/libs/metadataengine/dmetadata/dmetadata_libraw.cpp https://invent.kde.org/graphics/digikam/commit/f7521ff595caaee5a85fdd75354ac78a753e39dd
Git commit e662dbc0caea17d673cd84966655fe29fcc50828 by Gilles Caulier. Committed on 19/09/2020 at 10:58. Pushed by cgilles into branch 'master'. Use heap to create DRawInfo instance to prevent stack overflow Related: bug 426597 M +1 -1 core/libs/rawengine/drawdecoder.cpp M +12 -8 core/libs/rawengine/drawdecoder_p.cpp https://invent.kde.org/graphics/digikam/commit/e662dbc0caea17d673cd84966655fe29fcc50828
Git commit 605e6875ca4386291a0b681ec26db9db0b305757 by Gilles Caulier. Committed on 19/09/2020 at 11:18. Pushed by cgilles into branch 'master'. Use heap to create DRawInfo instance to prevent stack overflow Related: bug 426597 M +99 -99 core/libs/dngwriter/dngwriter_convert.cpp M +2 -0 core/libs/dngwriter/dngwriter_p.h https://invent.kde.org/graphics/digikam/commit/605e6875ca4386291a0b681ec26db9db0b305757
Git commit 96bc53a0682968ad3df03c698bf3e9bacb3a665d by Gilles Caulier. Committed on 19/09/2020 at 11:25. Pushed by cgilles into branch 'master'. Use heap to create DRawInfo instance to prevent stack overflow Related: bug 426597 M +2 -1 core/dplugins/dimg/raw/dimgrawloader.cpp M +4 -2 core/dplugins/generic/tools/htmlgallery/generator/galleryelementfunctor.cpp M +2 -1 core/libs/metadataengine/dmetadata/dmetadata_libraw.cpp M +2 -1 core/libs/rawengine/drawdecoder_p.cpp M +2 -1 core/libs/threadimageio/preview/previewtask.cpp M +16 -13 core/tests/rawengine/raw2png.cpp https://invent.kde.org/graphics/digikam/commit/96bc53a0682968ad3df03c698bf3e9bacb3a665d
Git commit a1deb248f5e457b5abef17d207cb16daef1237b9 by Gilles Caulier. Committed on 19/09/2020 at 11:36. Pushed by cgilles into branch 'master'. Use heap to create DRawDecoder instance to prevent stack overflow Related: bug 426597 M +1 -1 core/libs/progressmanager/workingwidget.cpp M +3 -2 core/libs/rawengine/drawdecoder.cpp M +1 -0 core/libs/threadimageio/preview/previewtask.cpp M +19 -6 core/tests/multithreading/myactionthread.cpp https://invent.kde.org/graphics/digikam/commit/a1deb248f5e457b5abef17d207cb16daef1237b9
Git commit 0507f9712e4cd76bcbe3aa4cec690ecb25e3fe9e by Gilles Caulier. Committed on 19/09/2020 at 11:42. Pushed by cgilles into branch 'master'. Use heap to create DMetadata instance to prevent stack overflow Related: bug 426597 M +2 -2 core/app/main/digikamapp.cpp M +1 -0 core/app/main/digikamapp_p.h M +11 -10 core/app/views/stack/mapwidgetview.cpp https://invent.kde.org/graphics/digikam/commit/0507f9712e4cd76bcbe3aa4cec690ecb25e3fe9e
Marcel, My last commits instanciate DRawInfo and DRawDecoder on heap, not on the stack. If ulimit fix low value on your system to allocate stack memory, this can crash the system (see report #426597 for details). I plan to patch all DMetadata and DMetaengine instances in the same way. But there are plenty ones everywhere, so this will take time. Gilles
Gilles, great work. This solved the problem of the sigfault. Anyway, I'm still not able to finish a full scan of my library. Again I'm running out of memory. It turned out there is still a leak or some resources that are not released before the scan is finished. I tried it twice.
Ah, <fr> "enfin" </fr>, some progress in this room (:=)))... Ok, let's me some time to check the static analyzer results. perhaps in my last commits i forget something to clear objects from heap memory. The changes are important and the error is always an human stuff. Gilles
MArcel, look the comment 14 here : https://bugs.kde.org/show_bug.cgi?id=426597#c14 It's clear, there is a heap memory corruption in libraw::tiff processor... Gilles
Git commit c3e43795e4bbe3be6b59692916b494a90d97c172 by Gilles Caulier. Committed on 19/09/2020 at 14:30. Pushed by cgilles into branch 'master'. Use heap to create DMetadata instance to prevent stack overflow M +6 -6 core/libs/threadimageio/preview/previewtask.cpp M +9 -5 core/libs/threadimageio/thumb/thumbnailcreator_engine.cpp M +1 -0 core/libs/threadimageio/thumb/thumbnailcreator_p.h https://invent.kde.org/graphics/digikam/commit/c3e43795e4bbe3be6b59692916b494a90d97c172
Git commit ac89e0f9a60ddde563c23e8db64da99cf5262d76 by Gilles Caulier. Committed on 19/09/2020 at 14:45. Pushed by cgilles into branch 'master'. Use heap to create DMetadata instance to prevent stack overflow M +24 -24 core/libs/dimg/dimg_metadata.cpp M +1 -0 core/libs/dimg/dimg_p.h M +2 -2 core/libs/dimg/dimg_transform.cpp M +4 -3 core/libs/dimg/filters/hotpixels/blackframelistviewitem.cpp M +4 -3 core/libs/dimg/filters/lens/lensfunfilter.cpp M +21 -16 core/libs/dimg/loaders/dimgloader.cpp https://invent.kde.org/graphics/digikam/commit/ac89e0f9a60ddde563c23e8db64da99cf5262d76
Git commit 45092f74196f6b97054dbeda6139b653bfc1c2f7 by Gilles Caulier. Committed on 19/09/2020 at 14:54. Pushed by cgilles into branch 'master'. Use heap to create DMetadata instance to prevent stack overflow M +21 -20 core/libs/dplugins/iface/dmetainfoiface.cpp M +6 -5 core/libs/jpegutils/jpegutils.cpp M +5 -4 core/libs/widgets/history/itemfiltershistorymodel.cpp https://invent.kde.org/graphics/digikam/commit/45092f74196f6b97054dbeda6139b653bfc1c2f7
Gilles are you sure? I don't have a single raw image and just one tiff. How could this leak come from a part I'm not using?I guess the the problem now must come from somewhere else.
yes, i 'm sure that libraw make glitch with memory, but i never said that it's the origin of your problem (:=)))... Gilles
Git commit 33b43acb76fff9c36925518bbb7bd27f23b5ff5b by Gilles Caulier. Committed on 19/09/2020 at 16:51. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +6 -0 core/dplugins/dimg/heif/dimgheifloader.cpp M +10 -5 core/dplugins/dimg/heif/dimgheifloader_save.cpp M +9 -5 core/dplugins/generic/tools/calendar/print/calpainter.cpp M +16 -15 core/libs/metadataengine/dmetadata/dmetadata_generic.cpp https://invent.kde.org/graphics/digikam/commit/33b43acb76fff9c36925518bbb7bd27f23b5ff5b
Git commit 765701869ec1b3506407bb6f3b790e43f3763a64 by Gilles Caulier. Committed on 19/09/2020 at 21:00. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +4 -3 core/dplugins/bqm/colors/iccconvert/iccconvert.cpp M +3 -2 core/dplugins/bqm/enhance/lensautofix/lensautofix.cpp M +10 -9 core/dplugins/bqm/metadata/assigntemplate/assigntemplate.cpp M +31 -30 core/dplugins/bqm/metadata/removemetadata/removemetadata.cpp M +64 -63 core/dplugins/bqm/metadata/timeadjust/timeadjust.cpp https://invent.kde.org/graphics/digikam/commit/765701869ec1b3506407bb6f3b790e43f3763a64
Git commit 1893c2a88b6d8ea40095c2ec05ea6a533a15924a by Gilles Caulier. Committed on 20/09/2020 at 03:08. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +3 -2 core/dplugins/dimg/jpeg2000/dimgjpeg2000loader_load.cpp M +39 -22 core/dplugins/dimg/tiff/dimgtiffloader_save.cpp M +14 -13 core/dplugins/generic/import/dscanner/saveimgthread.cpp M +10 -10 core/dplugins/generic/metadata/timeadjust/timeadjustdialog.cpp M +56 -55 core/dplugins/generic/metadata/timeadjust/timeadjusttask.cpp M +36 -36 core/dplugins/generic/tools/htmlgallery/generator/galleryelementfunctor.cpp M +3 -3 core/dplugins/generic/tools/printcreator/manager/advprinttask.cpp M +6 -7 core/dplugins/generic/tools/printcreator/wizard/advprintcaptionpage.cpp M +10 -9 core/dplugins/generic/tools/sendbymail/manager/imageresizejob.cpp M +3 -2 core/dplugins/generic/view/glviewer/glviewertexture.cpp M +11 -8 core/dplugins/generic/webservices/unified/manager/wsauthentication.cpp https://invent.kde.org/graphics/digikam/commit/1893c2a88b6d8ea40095c2ec05ea6a533a15924a
Git commit 86b911ae10d0f24ba5f2c9c637348605d8b53029 by Gilles Caulier. Committed on 20/09/2020 at 03:54. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +1 -0 core/libs/database/collection/collectionscanner_p.h M +8 -8 core/libs/database/collection/collectionscanner_utils.cpp M +148 -146 core/libs/dngwriter/dngwriter_convert.cpp M +10 -6 core/libs/fileactionmanager/fileworkeriface.cpp M +20 -19 core/libs/fileactionmanager/metadatahub.cpp M +7 -3 core/utilities/advancedrename/parser/options/cameranameoption.cpp M +6 -5 core/utilities/advancedrename/parser/options/metadataoption.cpp M +4 -3 core/utilities/imageeditor/core/editorcore.cpp M +4 -4 core/utilities/imageeditor/main/imagewindow.cpp M +1 -0 core/utilities/imageeditor/main/imagewindow_p.h M +15 -14 core/utilities/import/backend/cameracontroller.cpp M +26 -3 core/utilities/import/backend/gpcamera.cpp M +15 -14 core/utilities/import/backend/umscamera.cpp M +4 -3 core/utilities/queuemanager/manager/batchtool.cpp https://invent.kde.org/graphics/digikam/commit/86b911ae10d0f24ba5f2c9c637348605d8b53029
Git commit feb2f2aeb8b45dd6d4d8628ad588efe7f5183050 by Gilles Caulier. Committed on 20/09/2020 at 11:32. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +11 -2 core/libs/database/item/scanner/itemscanner_video.cpp M +4 -4 core/libs/fileactionmanager/metadatahub.cpp M +1 -1 core/libs/metadataengine/dmetadata/dmetadata.h M +1 -1 core/libs/metadataengine/dmetadata/dmetadatasettings.cpp M +1 -1 core/libs/metadataengine/dmetadata/dmetadatasettings.h M +1 -1 core/libs/metadataengine/dmetadata/dmetadatasettingscontainer.cpp M +1 -1 core/libs/metadataengine/dmetadata/dmetadatasettingscontainer.h M +8 -2 core/libs/properties/geolocation/itempropertiesgpstab.cpp M +17 -3 core/libs/properties/import/importitempropertiessidebar.cpp M +6 -5 core/libs/properties/itempropertiesmetadatatab.cpp M +8 -8 core/libs/properties/itempropertiessidebar.cpp M +4 -3 core/libs/properties/itempropertiessidebardb.cpp M +4 -3 core/libs/timeadjust/clockphotodialog.cpp M +8 -7 core/libs/widgets/metadata/exifwidget.cpp M +8 -7 core/libs/widgets/metadata/iptcwidget.cpp M +8 -7 core/libs/widgets/metadata/makernotewidget.cpp M +7 -6 core/libs/widgets/metadata/metadatapanel.cpp M +1 -1 core/libs/widgets/metadata/metadatawidget.cpp M +8 -7 core/libs/widgets/metadata/xmpwidget.cpp M +12 -12 core/showfoto/main/showfoto.cpp M +1 -0 core/showfoto/main/showfoto_p.h M +4 -3 core/showfoto/thumbbar/showfotothumbnailmodel.cpp https://invent.kde.org/graphics/digikam/commit/feb2f2aeb8b45dd6d4d8628ad588efe7f5183050
Git commit c3c15b7934c0faece918ff9ef16543a26909d33d by Gilles Caulier. Committed on 20/09/2020 at 12:05. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +3 -2 core/tests/filters/testlensfuniface.cpp M +4 -3 core/tests/geolocation/geoiface/demo/mainwindow.cpp M +9 -9 core/tests/metadataengine/applytagstest.cpp M +15 -15 core/tests/metadataengine/commentreadwritetest.cpp M +1 -1 core/tests/metadataengine/commentreadwritetest.h M +6 -6 core/tests/metadataengine/createxmpsidecartest.cpp M +16 -16 core/tests/metadataengine/erasemetadatatagtest.cpp M +2 -2 core/tests/metadataengine/loadfrombatest.cpp M +13 -13 core/tests/metadataengine/patchpreviewtest.cpp M +12 -12 core/tests/metadataengine/printiteminfotest.cpp M +5 -5 core/tests/metadataengine/printmetadatatest.cpp M +9 -9 core/tests/metadataengine/printtagslisttest.cpp M +16 -16 core/tests/metadataengine/ratingreadwritetest.cpp M +1 -1 core/tests/metadataengine/ratingreadwritetest.h M +7 -7 core/tests/metadataengine/setiptcpreviewtest.cpp M +15 -15 core/tests/metadataengine/setxmpfacetest.cpp M +23 -23 core/tests/metadataengine/tagsreadwritetest.cpp M +1 -1 core/tests/metadataengine/tagsreadwritetest.h M +15 -15 core/tests/metadataengine/usexmpsidecartest.cpp M +24 -23 core/tests/timestampupdate/timestampupdatetest.cpp https://invent.kde.org/graphics/digikam/commit/c3c15b7934c0faece918ff9ef16543a26909d33d
Git commit 115956282471cb03a45ea61a821da26a0df2d698 by Gilles Caulier. Committed on 20/09/2020 at 12:32. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +6 -6 core/dplugins/editor/colors/profileconversion/profileconversiontool.cpp M +4 -2 core/dplugins/editor/enhance/lensautofix/lensautofixtool.cpp M +6 -6 core/dplugins/generic/webservices/box/boxtalker.cpp M +6 -6 core/dplugins/generic/webservices/dropbox/dbtalker.cpp M +6 -6 core/dplugins/generic/webservices/facebook/fbwindow.cpp M +8 -8 core/dplugins/generic/webservices/filecopy/fctask.cpp M +8 -8 core/dplugins/generic/webservices/flickr/flickrtalker.cpp M +6 -6 core/dplugins/generic/webservices/google/gdrive/gdtalker.cpp M +7 -7 core/dplugins/generic/webservices/google/gphoto/gptalker.cpp M +10 -10 core/dplugins/generic/webservices/imgur/imgurimageslist.cpp M +8 -7 core/dplugins/generic/webservices/ipfs/ipfsimageslist.cpp M +8 -8 core/dplugins/generic/webservices/mediawiki/mediawikiwindow.cpp M +6 -6 core/dplugins/generic/webservices/onedrive/odtalker.cpp M +6 -6 core/dplugins/generic/webservices/piwigo/piwigotalker.cpp M +6 -6 core/dplugins/generic/webservices/rajce/rajcecommand.cpp M +6 -6 core/dplugins/generic/webservices/smugmug/smugwindow.cpp https://invent.kde.org/graphics/digikam/commit/115956282471cb03a45ea61a821da26a0df2d698
Git commit fc1026412d461c3922f430ab3b800323d2df98c7 by Gilles Caulier. Committed on 20/09/2020 at 12:36. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +24 -24 core/dplugins/generic/metadata/metadataedit/exif/exifadjust.cpp M +29 -29 core/dplugins/generic/metadata/metadataedit/exif/exifcaption.cpp M +36 -36 core/dplugins/generic/metadata/metadataedit/exif/exifdatetime.cpp M +50 -50 core/dplugins/generic/metadata/metadataedit/exif/exifdevice.cpp M +14 -14 core/dplugins/generic/metadata/metadataedit/exif/exifeditwidget.cpp M +28 -28 core/dplugins/generic/metadata/metadataedit/exif/exiflens.cpp M +18 -18 core/dplugins/generic/metadata/metadataedit/exif/exiflight.cpp M +11 -11 core/dplugins/generic/metadata/metadataedit/iptc/iptccategories.cpp M +18 -18 core/dplugins/generic/metadata/metadataedit/iptc/iptccontent.cpp M +23 -23 core/dplugins/generic/metadata/metadataedit/iptc/iptccredits.cpp M +11 -11 core/dplugins/generic/metadata/metadataedit/iptc/iptceditwidget.cpp M +35 -35 core/dplugins/generic/metadata/metadataedit/iptc/iptcenvelope.cpp M +8 -8 core/dplugins/generic/metadata/metadataedit/iptc/iptckeywords.cpp M +39 -39 core/dplugins/generic/metadata/metadataedit/iptc/iptcorigin.cpp M +37 -37 core/dplugins/generic/metadata/metadataedit/iptc/iptcproperties.cpp M +17 -17 core/dplugins/generic/metadata/metadataedit/iptc/iptcstatus.cpp M +8 -8 core/dplugins/generic/metadata/metadataedit/iptc/iptcsubjects.cpp M +11 -11 core/dplugins/generic/metadata/metadataedit/xmp/xmpcategories.cpp M +23 -23 core/dplugins/generic/metadata/metadataedit/xmp/xmpcontent.cpp M +66 -66 core/dplugins/generic/metadata/metadataedit/xmp/xmpcredits.cpp M +11 -11 core/dplugins/generic/metadata/metadataedit/xmp/xmpeditwidget.cpp M +8 -8 core/dplugins/generic/metadata/metadataedit/xmp/xmpkeywords.cpp M +56 -56 core/dplugins/generic/metadata/metadataedit/xmp/xmporigin.cpp M +23 -23 core/dplugins/generic/metadata/metadataedit/xmp/xmpproperties.cpp M +17 -17 core/dplugins/generic/metadata/metadataedit/xmp/xmpstatus.cpp M +8 -8 core/dplugins/generic/metadata/metadataedit/xmp/xmpsubjects.cpp https://invent.kde.org/graphics/digikam/commit/fc1026412d461c3922f430ab3b800323d2df98c7
Marcel, I patched all DMetadata/MetaEngine instance to use heap instead stack to prevent stack overflow. This will improve the stability on your computer ? Gilles
Git commit e79a1a3bd23e886cbb230395b588470dfd04c236 by Gilles Caulier. Committed on 20/09/2020 at 14:30. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +28 -26 core/dplugins/generic/tools/panorama/tasks/preprocesstask.cpp M +6 -6 core/dplugins/generic/webservices/google/gphoto/gptalker.cpp M +17 -13 core/dplugins/generic/webservices/google/gswindow.cpp M +8 -7 core/dplugins/generic/webservices/pinterest/ptalker.cpp M +8 -7 core/dplugins/generic/webservices/twitter/twittertalker.cpp M +22 -18 core/dplugins/generic/webservices/yandexfotki/yfwindow.cpp M +5 -5 core/libs/dialogs/imagedialog.cpp https://invent.kde.org/graphics/digikam/commit/e79a1a3bd23e886cbb230395b588470dfd04c236
Git commit b4589b0d2a0e31cb709da3554c0190c8d71f7b89 by Gilles Caulier. Committed on 20/09/2020 at 15:49. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +10 -10 core/libs/properties/itempropertiesmetadatatab.cpp M +1 -1 core/libs/widgets/metadata/metadatawidget.cpp https://invent.kde.org/graphics/digikam/commit/b4589b0d2a0e31cb709da3554c0190c8d71f7b89
Git commit 10e2ecab81ecc4f45989f82987021a34dbb3558d by Gilles Caulier. Committed on 20/09/2020 at 15:58. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +3 -3 core/libs/widgets/metadata/exifwidget.cpp M +3 -3 core/libs/widgets/metadata/iptcwidget.cpp M +3 -3 core/libs/widgets/metadata/makernotewidget.cpp M +3 -3 core/libs/widgets/metadata/xmpwidget.cpp https://invent.kde.org/graphics/digikam/commit/10e2ecab81ecc4f45989f82987021a34dbb3558d
Git commit 3f17d2d4ee8f2877ae6036387ad1360beac1ca84 by Gilles Caulier. Committed on 20/09/2020 at 16:10. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +2 -2 core/libs/widgets/metadata/exifwidget.cpp M +2 -2 core/libs/widgets/metadata/iptcwidget.cpp M +2 -2 core/libs/widgets/metadata/makernotewidget.cpp M +12 -5 core/libs/widgets/metadata/metadatawidget.cpp M +5 -5 core/libs/widgets/metadata/metadatawidget.h M +3 -3 core/libs/widgets/metadata/xmpwidget.cpp https://invent.kde.org/graphics/digikam/commit/3f17d2d4ee8f2877ae6036387ad1360beac1ca84
(In reply to caulier.gilles from comment #145) > Marcel, > > I patched all DMetadata/MetaEngine instance to use heap instead stack to > prevent stack overflow. > > This will improve the stability on your computer ? > > Gilles I tried the version directly after this comment. Unfortunately it still leaks memory, but not much. This time my computer run out of memory after around 1.5h. I now start the test again with your most recent commits. I could upload the system monitor picture during detection. What I don't understand is that during face detection 9 threads are running with full load on my dual core cpu with 4 tubes. From compiling a program it sait that the best would be the tube number or the tube number +1. Where is my misunderstanding?
Marcel, You lost memory a little bit, but the infamous crash in Preview thread disappear. Right ? If yes, we will trying to discover the source of the problem for this memory. Next stage for me is to add a new compilation option to use ASAN tool: https://en.wikipedia.org/wiki/AddressSanitizer About multithread while face detection, i'm not sure. If i remmeber, Nghia report to me that multicore support in face detection have been disabled for performance reasons, as low level API is not re-entrant. Perhaps Nghia can provide more details for this point. Gilles
Git commit a8f47f320928428f102df5ff40a86c3d41184c89 by Gilles Caulier. Committed on 20/09/2020 at 18:16. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +6 -6 core/libs/properties/geolocation/itempropertiesgpstab.cpp M +1 -1 core/libs/properties/geolocation/itempropertiesgpstab.h M +17 -3 core/libs/properties/import/importitempropertiessidebar.cpp M +5 -5 core/libs/properties/import/importitempropertiestab.cpp M +1 -1 core/libs/properties/import/importitempropertiestab.h M +2 -2 core/libs/properties/itempropertiesmetadatatab.cpp M +3 -3 core/libs/properties/itempropertiesmetadatatab.h M +1 -1 core/libs/properties/itempropertiessidebardb.cpp https://invent.kde.org/graphics/digikam/commit/a8f47f320928428f102df5ff40a86c3d41184c89
(In reply to caulier.gilles from comment #151) > Marcel, > > You lost memory a little bit, but the infamous crash in Preview thread > disappear. Right ? > > If yes, we will trying to discover the source of the problem for this > memory. Next stage for me is to add a new compilation option to use ASAN > tool: > > https://en.wikipedia.org/wiki/AddressSanitizer > > About multithread while face detection, i'm not sure. If i remmeber, Nghia > report to me that multicore support in face detection have been disabled for > performance reasons, as low level API is not re-entrant. > > Perhaps Nghia can provide more details for this point. > > Gilles Hi Gilles, It's not quite what you said. I disabled multiple face extractors in faces engine, which means for now, in faccial recognition, there is only one FaceExtractor being used. This avoid multiple memory allocation for faical recognition. However, with only one FaceExtractor, the facial recognition process still runs multi-threading by using cv::parallel_for_ function. It's implemented in file : https://invent.kde.org/graphics/digikam/-/blob/master/core/libs/facesengine/recognition/opencv-dnn/opencvdnnfacerecognizer.cpp , at lines 465 and 505. If you want to desactivate it for a test run, just replace these line with a normal loop. Nghia
Nghia, i think Marcel only play with face detection, not face recognition for the moment. FaceExtractor in only relevant with face recognition workflow, right ? Gilles
(In reply to caulier.gilles from comment #154) Yes, only one FaceExtractor is allocated at the init sequence of digikam. After that, the multi-threading only envoked in the recognition process.
Git commit cd24540e6da7170b56e6a2b9a2de3138d0769969 by Maik Qualmann. Committed on 20/09/2020 at 20:01. Pushed by mqualmann into branch 'master'. check if DImg data equal M +4 -1 core/libs/dimg/dimg_data.cpp https://invent.kde.org/graphics/digikam/commit/cd24540e6da7170b56e6a2b9a2de3138d0769969
Git commit 2b8a165d227d8cb9f90c21db745bc699d254ce44 by Gilles Caulier. Committed on 20/09/2020 at 20:53. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +1 -1 core/dplugins/bqm/enhance/lensautofix/lensautofix.cpp M +1 -2 core/dplugins/editor/enhance/lensautofix/lensautofixtool.cpp M +1 -1 core/libs/dimg/dimg.h M +13 -6 core/libs/dimg/filters/lens/lensfuncameraselector.cpp M +3 -2 core/libs/dimg/filters/lens/lensfuncameraselector.h M +9 -9 core/libs/dimg/filters/lens/lensfuniface.cpp M +1 -1 core/libs/dimg/filters/lens/lensfuniface.h M +5 -5 core/libs/fileactionmanager/metadatahub.h M +1 -1 core/tests/filters/testlensfuniface.cpp https://invent.kde.org/graphics/digikam/commit/2b8a165d227d8cb9f90c21db745bc699d254ce44
Git commit ef51f605528649e9f509bd37d2710409f3bf83f0 by Gilles Caulier. Committed on 20/09/2020 at 21:32. Pushed by cgilles into branch 'master'. Add new compilation option to enable compiler sanitizers ASAN and UBSAN Related: bug 426597 M +1 -0 Mainpage.dox M +6 -1 core/CMakeLists.txt M +3 -1 core/cmake/modules/MacroCompiler.cmake https://invent.kde.org/graphics/digikam/commit/ef51f605528649e9f509bd37d2710409f3bf83f0
I run the face detection over night and run out of memory again, but it's definitely not the seg fault from the Preview process that crashes to program. This part is solved. Thank you Gilles. It's true, I'm just playing with face detection and not recognition. I'm asking about the worker threads as on my system this threads just use more memory and don't increase the speed of the process. Probably there is some space for lower memory demand. I started now compiling with clang to see if the compiler has any effect on that memory leak.
Marcel, This morning i append ASAN support. There is a new cmake compilation option: https://invent.kde.org/graphics/digikam/-/blob/master/Mainpage.dox#L461 You need to install libasan-devel for linking stage first (i need to add cmake check dependencies, it's in my TODO list). Warning: ASAN require more memory to check... memory allocations (:-)))... https://en.wikipedia.org/wiki/AddressSanitizer There are plenty of checker to enable, but for the moment i just turn on the most common: address sanitizer. I can see a linking failure with other ones. This need investigations. Gilles
Git commit edcab64cdca0ae1800bab455724b9753dffd000c by Maik Qualmann. Committed on 21/09/2020 at 06:05. Pushed by mqualmann into branch 'master'. for a test, remove myself from the loading listener beforehand M +5 -5 core/libs/threadimageio/fileio/loadsavetask.cpp M +38 -36 core/libs/threadimageio/preview/previewtask.cpp M +4 -4 core/libs/threadimageio/thumb/thumbnailtask.cpp https://invent.kde.org/graphics/digikam/commit/edcab64cdca0ae1800bab455724b9753dffd000c
Bug news, on my test with clang the bad old error occurred again after a few minutes of scanning: Thread 47 "Thread (pooled)" received signal SIGSEGV, Segmentation fault. #1 0x00007ffff6a852b5 in Digikam::PreviewLoadingTask::execute (this=0xaec5a80) at /home/marcel/Repos/digikam/core/libs/threadimageio/preview/previewtask.cpp:321 I will try running it with asan. If my memory is too small I will increase the swap for this test. I didn't increased swap so far because I think the scanning must work with 8GB of ram.
I now included the changes from Maik as well. My test runs now for over 40 minutes. Hey, it didn't crash and I can't see any significant increase in memory. Probably the last changes from Maik finally solved this bug. Thank you all for working on this bug. @Maik: What was the problem exactly? I will confirm the result after the scan finishes.
Created attachment 131836 [details] Face detection process in htop I tested it on my system and it worked for more than 5 hours. After that Digikam was killed by another process. The log said only "killed". Despite that crash I consider this bug as solved. In this 5 hours of scanning I experiences some strange behavior. Memory increased sometimes in very small steps until it reached a limit, then in a few seconds 3GB of RAM was released again and all started from the beginning. See in the picture htop.png my memory consumption cut down to threads. Why are there so many threads using so much memory?
Marcel, In htop, you can setup to turn on the thread names. This can help to identify the thread functions (i hope that all QThread instances are explicitly identified with Linux Kernel. Typically, the object name is used for that. I use this powerful features in my office which run under Linux only. I suspect that all these threads are used to register faces in database, but i'm not 100% sure. And the Question is why this happen only few times after a long running time... Gilles
QThread object name visible as Linux process : the explaination are there : https://stackoverflow.com/questions/19854932/is-there-a-portable-way-to-give-thread-name-with-qt Gilles
Gilles, thank you for this tip with htop. Great to know about such debug possibilities. Unfortunately the name of about half of the QThread's have the name "Thread (pooled)". About the asan. It now works on my computer, but it needed a workaround in your script. find_library(ASAN_LIBRARIES NAMES asan) did not find anything on my system. On Ubuntu the asan is installed by default with the gcc. See the comment from yugr on https://stackoverflow.com/questions/37970758/how-to-use-addresssanitizer-with-gcc. Should I provide you a patch with a fix? I now run the scanning again over night with asan enabled.
Git commit 090bfe8ffd2ec534a0f2a100e4e2e4c3083e300b by Maik Qualmann. Committed on 21/09/2020 at 19:47. Pushed by mqualmann into branch 'master'. reduce sent face packets M +1 -1 core/utilities/facemanagement/threads/facepreviewloader.cpp https://invent.kde.org/graphics/digikam/commit/090bfe8ffd2ec534a0f2a100e4e2e4c3083e300b
Marcel, yes sure a patch is always welcome. Gilles
Git commit c12a37ca883329d746c74fa54b94cd39eb8324c9 by Maik Qualmann. Committed on 21/09/2020 at 20:55. Pushed by mqualmann into branch 'master'. remove the created thumbnails threads because they are unused M +0 -5 core/utilities/facemanagement/threads/facepipeline.cpp https://invent.kde.org/graphics/digikam/commit/c12a37ca883329d746c74fa54b94cd39eb8324c9
Git commit 464c7cf5d10f84bacfbbe98fe4fa6193e1d64773 by Gilles Caulier. Committed on 22/09/2020 at 03:09. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +15 -9 core/libs/jpegutils/jpegutils.cpp M +6 -1 core/libs/jpegutils/jpegutils.h https://invent.kde.org/graphics/digikam/commit/464c7cf5d10f84bacfbbe98fe4fa6193e1d64773
Git commit 198dc96fe39c17c038b4b0782fca81cc199bbdc7 by Gilles Caulier. Committed on 22/09/2020 at 03:52. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +3 -3 core/libs/database/item/scanner/itemscanner.cpp M +2 -2 core/libs/database/item/scanner/itemscanner_file.cpp M +3 -3 core/libs/database/item/scanner/itemscanner_history.cpp M +6 -0 core/libs/database/item/scanner/itemscanner_p.cpp M +2 -1 core/libs/database/item/scanner/itemscanner_p.h M +10 -10 core/libs/database/item/scanner/itemscanner_photo.cpp M +6 -6 core/libs/database/item/scanner/itemscanner_video.cpp https://invent.kde.org/graphics/digikam/commit/198dc96fe39c17c038b4b0782fca81cc199bbdc7
Git commit aa1de72a075ef92c753b90021488e555b246ae42 by Gilles Caulier. Committed on 22/09/2020 at 03:38. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +25 -23 core/dplugins/generic/tools/panorama/tasks/copyfilestask.cpp M +0 -2 core/dplugins/generic/tools/panorama/tasks/copyfilestask.h https://invent.kde.org/graphics/digikam/commit/aa1de72a075ef92c753b90021488e555b246ae42
Git commit a2f03025488e56fc2c1ae2f3f2e48388b09c0e0f by Gilles Caulier. Committed on 22/09/2020 at 03:39. Pushed by cgilles into branch 'master'. Use heap to create MetaEngine instance to prevent stack overflow M +20 -13 core/dplugins/generic/metadata/geolocationedit/kmlexport/kmlexport.cpp M +1 -1 core/dplugins/generic/metadata/geolocationedit/kmlexport/kmlexport.h https://invent.kde.org/graphics/digikam/commit/a2f03025488e56fc2c1ae2f3f2e48388b09c0e0f
Created attachment 131853 [details] heap-use-after-free error I run Digikam with the address sanitizer (fbf127ecf30867613d3fe247be85b4f09533e046). Indeed there was an error in the previewtask. See the output from asan.
Git commit 51e1ccaa60e71f253bb7c760a3f5c678f3cbc902 by Gilles Caulier. Committed on 22/09/2020 at 05:33. Pushed by cgilles into branch 'master'. move all implmentations to cpp files. Header must provide declarations only always provide a default destructor in implementations. M +4 -0 core/libs/threadimageio/preview/previewloadthread.cpp M +23 -7 core/libs/threadimageio/preview/previewloadthread.h M +14 -1 core/libs/threadimageio/preview/previewsettings.cpp M +4 -3 core/libs/threadimageio/preview/previewsettings.h M +10 -0 core/libs/threadimageio/preview/previewtask.cpp M +2 -5 core/libs/threadimageio/preview/previewtask.h https://invent.kde.org/graphics/digikam/commit/51e1ccaa60e71f253bb7c760a3f5c678f3cbc902
Git commit 5ce6276093c8a636d6844b060cb9020f01c20f18 by Gilles Caulier. Committed on 22/09/2020 at 13:12. Pushed by cgilles into branch 'master'. move all implmentations to cpp files. Header must provide declarations only always provide a default destructor in implementations. M +70 -3 core/libs/threadimageio/fileio/loadsavetask.cpp M +35 -62 core/libs/threadimageio/fileio/loadsavetask.h https://invent.kde.org/graphics/digikam/commit/5ce6276093c8a636d6844b060cb9020f01c20f18
Created attachment 131868 [details] Address Sanitizer error on PreviewLoadingTask Gilles, I just checked your changes on the PreviewLoadingTask. I got an error on the PreviewLoadingTask with the address sanitizer.
Hi Marcel, This is interresting, because my change are pure cosmetic for the moment. Typically i moved all class method implementation from headers to cpp files, and append missing default constructors or destructors. For this last point, C++ norm is clear : "Defining a class without a constructor but with a destructor is perfectly legal C++, but is not recommended. Your compiler will try to generate a default constructor for you, which will automatically call the default constructor of all members in your class. You can explicitly request this by marking your constructor as default, but you lose finer control over the instantiation of your class, and cannot do anything but default construct all members of your class." Gilles
Git commit d4b496877424462cc01c1860bf2bbc544cdb9585 by Gilles Caulier. Committed on 22/09/2020 at 14:49. Pushed by cgilles into branch 'master'. add default contructor and destructor move implementations to .cpp M +24 -2 core/libs/threadimageio/fileio/loadingcache.cpp M +7 -4 core/libs/threadimageio/fileio/loadingcache.h https://invent.kde.org/graphics/digikam/commit/d4b496877424462cc01c1860bf2bbc544cdb9585
Git commit 9f89a5264d45d9cffdd331b7a5d98a4dbde45b51 by Maik Qualmann. Committed on 22/09/2020 at 19:21. Pushed by mqualmann into branch 'master'. preview loading tasks with new stop logic M +20 -51 core/libs/threadimageio/fileio/loadsavetask.cpp M +20 -20 core/libs/threadimageio/preview/previewtask.cpp M +14 -14 core/libs/threadimageio/thumb/thumbnailtask.cpp https://invent.kde.org/graphics/digikam/commit/9f89a5264d45d9cffdd331b7a5d98a4dbde45b51
Git commit 8333a8f06f714950729d7a85d87c9eb9d0cc4a6c by Gilles Caulier. Committed on 23/09/2020 at 05:53. Pushed by cgilles into branch 'master'. We can disable copy constructor and equality operator with these classes M +6 -0 core/libs/threadimageio/preview/previewloadthread.h M +6 -0 core/libs/threadimageio/preview/previewtask.h https://invent.kde.org/graphics/digikam/commit/8333a8f06f714950729d7a85d87c9eb9d0cc4a6c
Git commit 1f11358b3918a16d7576f6f8a80b644544784975 by Gilles Caulier. Committed on 23/09/2020 at 08:46. Pushed by cgilles into branch 'master'. We can disable copy constructor and equality operator with these classes M +24 -0 core/libs/threadimageio/fileio/loadsavetask.h https://invent.kde.org/graphics/digikam/commit/1f11358b3918a16d7576f6f8a80b644544784975
Git commit 5ca5eb5260994a4b4200c1626a31b78aeef02858 by Gilles Caulier. Committed on 23/09/2020 at 08:55. Pushed by cgilles into branch 'master'. We can disable copy constructor and equality operator with these classes M +21 -3 core/libs/threadimageio/fileio/loadingcache.h https://invent.kde.org/graphics/digikam/commit/5ca5eb5260994a4b4200c1626a31b78aeef02858
Git commit d0cf9690e91b59a98e152985535912516eb1e392 by Gilles Caulier. Committed on 23/09/2020 at 09:00. Pushed by cgilles into branch 'master'. This class is just a static methods container. No need construtor, destructor, copy constructor and equality operator M +9 -0 core/libs/threadimageio/fileio/loadingcacheinterface.h https://invent.kde.org/graphics/digikam/commit/d0cf9690e91b59a98e152985535912516eb1e392
Git commit 1ac2776d8ccee771a09dcb4ee4ec1699d7eaf65a by Gilles Caulier. Committed on 23/09/2020 at 09:12. Pushed by cgilles into branch 'master'. We can disable copy constructor and equality operator with these classes Add missing destructor M +4 -0 core/libs/threadimageio/engine/managedloadsavethread.h M +4 -0 core/libs/threadimageio/engine/sharedloadsavethread.cpp M +7 -0 core/libs/threadimageio/engine/sharedloadsavethread.h https://invent.kde.org/graphics/digikam/commit/1ac2776d8ccee771a09dcb4ee4ec1699d7eaf65a
Git commit 8b8aae7b266b94181239ed6e559deef4a4512f6c by Gilles Caulier. Committed on 23/09/2020 at 09:27. Pushed by cgilles into branch 'master'. We can disable copy constructor and equality operator with these classes M +1 -1 core/libs/threadimageio/CMakeLists.txt R +0 -0 core/libs/threadimageio/thumb/thumbnailcreator_basic.cpp [from: core/libs/threadimageio/thumb/thumbnailbasic.cpp - 100% similarity] M +12 -0 core/libs/threadimageio/thumb/thumbnailloadthread.h M +6 -0 core/libs/threadimageio/thumb/thumbnailtask.h https://invent.kde.org/graphics/digikam/commit/8b8aae7b266b94181239ed6e559deef4a4512f6c
Git commit f08d194f50b41c6d68fbb0855db20f807e5e7bfd by Gilles Caulier. Committed on 23/09/2020 at 10:10. Pushed by cgilles into branch 'master'. We can disable copy constructor and equality operator with these classes M +8 -1 core/libs/threads/dynamicthread.h M +12 -0 core/libs/threads/parallelworkers.h M +6 -0 core/libs/threads/threadmanager.h M +4 -0 core/libs/threads/workerobject.h https://invent.kde.org/graphics/digikam/commit/f08d194f50b41c6d68fbb0855db20f807e5e7bfd
Git commit 7600efe92d5979545189649229f01b4568361485 by Gilles Caulier. Committed on 23/09/2020 at 15:54. Pushed by cgilles into branch 'master'. We can disable copy constructor and equality operator with these classes. Add missing destructor M +4 -0 core/utilities/facemanagement/workers/databasewriter.cpp M +7 -0 core/utilities/facemanagement/workers/databasewriter.h M +6 -0 core/utilities/facemanagement/workers/detectionworker.h M +6 -0 core/utilities/facemanagement/workers/recognitionworker.h https://invent.kde.org/graphics/digikam/commit/7600efe92d5979545189649229f01b4568361485
Git commit cd1b6f82957813746a3f44a0251c1decab4790d7 by Gilles Caulier. Committed on 24/09/2020 at 01:56. Pushed by cgilles into branch 'master'. We can disable copy constructor and equality operator with these classes. Add missing destructor M +6 -0 core/utilities/facemanagement/threads/facepipeline.h M +33 -0 core/utilities/facemanagement/threads/facepipelinepackage.cpp M +10 -9 core/utilities/facemanagement/threads/facepipelinepackage.h M +10 -3 core/utilities/facemanagement/threads/facepreviewloader.cpp M +7 -0 core/utilities/facemanagement/threads/facepreviewloader.h M +6 -0 core/utilities/facemanagement/threads/parallelpipes.h M +16 -12 core/utilities/facemanagement/threads/scanstatefilter.cpp M +7 -0 core/utilities/facemanagement/threads/scanstatefilter.h https://invent.kde.org/graphics/digikam/commit/cd1b6f82957813746a3f44a0251c1decab4790d7
Git commit 04ca7557c15c4c7ce2cca005d054301d6d192562 by Gilles Caulier. Committed on 24/09/2020 at 02:04. Pushed by cgilles into branch 'master'. We can disable copy constructor and equality operator with these classes. Add missing destructor M +6 -0 core/utilities/facemanagement/items/facegroup.h M +4 -0 core/utilities/facemanagement/items/faceitem.cpp M +7 -0 core/utilities/facemanagement/items/faceitem.h https://invent.kde.org/graphics/digikam/commit/04ca7557c15c4c7ce2cca005d054301d6d192562
Git commit 7ed7c3ae44b23e1f5f5da0fe7bf195bf2b325fe1 by Gilles Caulier. Committed on 24/09/2020 at 02:15. Pushed by cgilles into branch 'master'. We can disable copy constructor and equality operator with these classes. Add missing destructor M +11 -7 core/utilities/facemanagement/database/faceitemretriever.cpp M +2 -0 core/utilities/facemanagement/database/faceitemretriever.h M +19 -18 core/utilities/facemanagement/database/faceutils.cpp M +6 -0 core/utilities/facemanagement/database/faceutils.h https://invent.kde.org/graphics/digikam/commit/7ed7c3ae44b23e1f5f5da0fe7bf195bf2b325fe1
Maik, I experiment the clang-tidy "linter" like analyzer with digiKam code and i found this report : clang-tidy -header-filter=^/home/gilles/Documents/GIT/7.x/build/.* -p=/home/gilles/Documents/GIT/7.x/build /home/gilles/Documents/GIT/7.x/core/libs/threads/workerobject.cpp /home/gilles/Documents/GIT/7.x/core/libs/threads/workerobject.cpp:247:5: warning: Call to virtual function during destruction [clang-analyzer-optin.cplusplus.VirtualCall] this->aboutToDeactivate(); ^ /home/gilles/Documents/GIT/7.x/core/libs/threads/workerobject.cpp:73:5: note: This destructor of an object of type '~WorkerObject' has not returned when the virtual method was called shutDown(); ^ /home/gilles/Documents/GIT/7.x/core/libs/threads/workerobject.cpp:73:5: note: Calling 'WorkerObject::shutDown' /home/gilles/Documents/GIT/7.x/core/libs/threads/workerobject.cpp:85:5: note: Calling 'WorkerObject::deactivate' deactivate(PhaseOut); ^ /home/gilles/Documents/GIT/7.x/core/libs/threads/workerobject.cpp:232:9: note: Control jumps to 'case Running:' at line 235 switch (d->state) ^ /home/gilles/Documents/GIT/7.x/core/libs/threads/workerobject.cpp:237:17: note: Execution continues on line 213 break; ^ /home/gilles/Documents/GIT/7.x/core/libs/threads/workerobject.cpp:247:5: note: Call to virtual function during destruction this->aboutToDeactivate(); ^ More info : https://clang.llvm.org/extra/clang-tidy/ Gilles
Yes, calling virtual functions in the constructor / destructor can cause problems. I also think that "this->aboutToDeactivate()" now never calls the derived class through "this". I'm testing it out tonight. I think we can mark the derived class with the "final" keyword to prevent problems. Maik
Right Maik, I fixed some this-> entry in source code in the pass after a large cppcheck report about dynamic binding problem. This one is a fix by me. You are right, instead to use this->, we must use final-> c++11 keyword. Note : all dynamic binding fixed are annotated with a "NOTE: use dynamic binding" comment in source code. It's easy to double check all entries to see if this-> or final-> must be used at these places (32 entries). grep -r "NOTE: use dynamic" core/* ../../core/app/date/ddateedit.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/app/date/ddateedit.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/app/date/ddateedit.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/app/items/views/digikamitemview.cpp: // --- NOTE: use dynamic binding as slotSetupChanged() is a virtual method which can be re-implemented in derived classes. ../../core/app/items/thumbbar/itemthumbnailbar.cpp: // --- NOTE: use dynamic binding as slotSetupChanged() is a virtual slot which can be re-implemented in derived classes. ../../core/dplugins/rawimport/native/rawpostprocessing.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/libs/models/abstractalbummodel.cpp: // --- NOTE: use dynamic binding as all slots above are virtual methods which can be re-implemented in derived classes. ../../core/libs/database/models/itemmodel.cpp: // --- NOTE: use dynamic binding as slotImageChange() is a virtual slot which can be re-implemented in derived classes. ../../core/libs/database/models/itemmodel.cpp: // --- NOTE: use dynamic binding as slotImageTagChange() is a virtual slot which can be re-implemented in derived classes. ../../core/libs/dimg/filters/dimgthreadedfilter.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/libs/dimg/filters/raw/rawprocessingfilter.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/libs/dimg/filters/greycstoration/greycstorationfilter.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/libs/dimg/filters/greycstoration/greycstorationfilter.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/libs/widgets/itemview/itemviewcategorized.cpp: // --- NOTE: use dynamic binding as slots below are virtual methods which can be re-implemented in derived classes. ../../core/libs/widgets/layout/sidebar.cpp: // --- NOTE: use dynamic binding as slotClicked() is a virtual method which can be re-implemented in derived classes. ../../core/libs/widgets/metadata/subjectwidget.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/libs/widgets/metadata/subjectwidget.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/libs/properties/itempropertiessidebar.cpp: // --- NOTE: use dynamic binding as slotChangedTab() is a virtual method which can be re-implemented in derived classes. ../../core/libs/properties/import/importitempropertiessidebar.cpp: // --- NOTE: use dynamic binding as slotChangedTab() is a virtual method which can be re-implemented in derived classes. ../../core/libs/album/widgets/albumselectcombobox.cpp: // --- NOTE: use dynamic binding as updateText() is a virtual slot which can be re-implemented in derived classes. ../../core/libs/dialogs/infodlg.cpp: // --- NOTE: use dynamic binding as slotCopy2ClipBoard() is a virtual slot which can be re-implemented in derived classes. ../../core/libs/metadataengine/engine/metaengine.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/libs/threads/workerobject.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/showfoto/thumbbar/showfotothumbnailbar.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/utilities/maintenance/maintenancetool.cpp: // --- NOTE: use dynamic binding as slotCancel() is a virtual method which can be re-implemented in derived classes. ../../core/utilities/imageeditor/editor/editortool.cpp: // --- NOTE: use dynamic binding as slotPreview() is a virtual method which can be re-implemented in derived classes. ../../core/utilities/setup/metadata/namespaceeditdlg.cpp: // --- NOTE: use dynamic binding as slots below are virtual method which can be re-implemented in derived classes. ../../core/utilities/import/views/importiconview.cpp: // --- NOTE: use dynamic binding as slotSetupChanged() is a virtual method which can be re-implemented in derived classes. ../../core/utilities/import/views/importthumbnailbar.cpp: // --- NOTE: use dynamic binding as slotSetupChanged() is a virtual method which can be re-implemented in derived classes. ../../core/utilities/geolocation/geoiface/tiles/abstractmarkertiler.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. ../../core/utilities/geolocation/geoiface/bookmark/bookmarksmenu.cpp: // --- NOTE: use dynamic binding as slotAboutToShow() is a virtual method which can be re-implemented in derived classes. ../../core/utilities/geolocation/geoiface/bookmark/bookmarksmenu.cpp: // NOTE: use dynamic binding as this virtual method can be re-implemented in derived classes. Gilles
I specifically debugged this WorkerObject class. Even if the analysis tools report an error here, everything in the order of destruction is OK. Right, the derived class is destroyed first. But then the "aboutToDeactivate()" function falls back to the base class and is then called during the destruction. Since the function is not implemented as a pure virtual function, everything is OK. The "this" pointer has no influence on the derived or basis class execution. Maik
Git commit c8721ba35c4b99cfed8696d2f7a01f2a8a66bac2 by Gilles Caulier. Committed on 25/09/2020 at 03:42. Pushed by cgilles into branch 'master'. Face engine: remove dead codes aka Eigen face recognition and Fisher face recognition algorithms. M +3 -20 core/libs/facesengine/CMakeLists.txt M +1 -1 core/libs/facesengine/TODO M +0 -28 core/libs/facesengine/facedb/facedb.h D +0 -204 core/libs/facesengine/facedb/facedb_eigen.cpp D +0 -87 core/libs/facesengine/facedb/facedb_fisher.cpp M +0 -8 core/libs/facesengine/facedb/facedb_p.h M +0 -3 core/libs/facesengine/recognition/README M +0 -6 core/libs/facesengine/recognition/facialrecognition_wrapper_p.cpp M +0 -13 core/libs/facesengine/recognition/facialrecognition_wrapper_p.h M +0 -8 core/libs/facesengine/recognition/facialrecognition_wrapper_training.cpp D +0 -187 core/libs/facesengine/recognition/opencv-eigenfaces/eigenfacemodel.cpp D +0 -107 core/libs/facesengine/recognition/opencv-eigenfaces/eigenfacemodel.h D +0 -253 core/libs/facesengine/recognition/opencv-eigenfaces/facerec_eigenborrowed.cpp D +0 -143 core/libs/facesengine/recognition/opencv-eigenfaces/facerec_eigenborrowed.h D +0 -201 core/libs/facesengine/recognition/opencv-eigenfaces/opencveigenfacerecognizer.cpp D +0 -91 core/libs/facesengine/recognition/opencv-eigenfaces/opencveigenfacerecognizer.h D +0 -311 core/libs/facesengine/recognition/opencv-fisherfaces/facerec_fisherborrowed.cpp D +0 -143 core/libs/facesengine/recognition/opencv-fisherfaces/facerec_fisherborrowed.h D +0 -184 core/libs/facesengine/recognition/opencv-fisherfaces/fisherfacemodel.cpp D +0 -104 core/libs/facesengine/recognition/opencv-fisherfaces/fisherfacemodel.h D +0 -197 core/libs/facesengine/recognition/opencv-fisherfaces/opencvfisherfacerecognizer.cpp D +0 -91 core/libs/facesengine/recognition/opencv-fisherfaces/opencvfisherfacerecognizer.h R +0 -0 core/libs/facesengine/recognition/opencv-lbph/opencv_face.cpp [from: core/libs/facesengine/recognition/opencv-face/opencv_face.cpp - 100% similarity] R +0 -0 core/libs/facesengine/recognition/opencv-lbph/opencv_face.h [from: core/libs/facesengine/recognition/opencv-face/opencv_face.h - 100% similarity] https://invent.kde.org/graphics/digikam/commit/c8721ba35c4b99cfed8696d2f7a01f2a8a66bac2
Git commit 65c9aa3d4fa6eea92409f786a0ba786332777682 by Gilles Caulier. Committed on 25/09/2020 at 04:05. Pushed by cgilles into branch 'master'. Face Engine: remove dead code aka flandmarkaligner algorithm M +0 -2 core/libs/facesengine/CMakeLists.txt D +0 -126 core/libs/facesengine/alignment/flandmark/flandmarkaligner.cpp D +0 -61 core/libs/facesengine/alignment/flandmark/flandmarkaligner.h https://invent.kde.org/graphics/digikam/commit/65c9aa3d4fa6eea92409f786a0ba786332777682
Git commit 880c377630be3393779185e1d1297e4957ff595b by Gilles Caulier. Committed on 25/09/2020 at 04:15. Pushed by cgilles into branch 'master'. Face Engine: move dead code not exported and only used in an experimental preprocessing CLI test tool. M +0 -4 core/libs/facesengine/CMakeLists.txt M +6 -1 core/tests/facesengine/CMakeLists.txt R +2 -5 core/tests/facesengine/preprocess/preprocess.cpp [from: core/tests/facesengine/preprocess.cpp - 095% similarity] R +0 -0 core/tests/facesengine/preprocess/tantriggspreprocessor.cpp [from: core/libs/facesengine/preprocessing/tantriggs/tantriggspreprocessor.cpp - 100% similarity] R +0 -0 core/tests/facesengine/preprocess/tantriggspreprocessor.h [from: core/libs/facesengine/preprocessing/tantriggs/tantriggspreprocessor.h - 100% similarity] https://invent.kde.org/graphics/digikam/commit/880c377630be3393779185e1d1297e4957ff595b
Git commit b6a240f95754eaa188f3aa97c59fdbd9785daa91 by Gilles Caulier. Committed on 25/09/2020 at 05:38. Pushed by cgilles into branch 'master'. Face Engine: remove dead code stage 2: Switch definitively to pure DNN detection and recognition. Remove all OpenCV/LBPH based algorithms. M +19 -61 core/libs/facesengine/CMakeLists.txt M +1 -1 core/libs/facesengine/TODO M +0 -1 core/libs/facesengine/detection/README M +6 -152 core/libs/facesengine/detection/facedetector.cpp M +1 -0 core/libs/facesengine/detection/opencv-dnn/dnnfacedetectorbase.h D +0 -658 core/libs/facesengine/detection/opencv-face/opencvfacedetector.cpp D +0 -128 core/libs/facesengine/detection/opencv-face/opencvfacedetector.h D +0 -194 core/libs/facesengine/detection/opencv-face/opencvfacedetector_p.cpp D +0 -170 core/libs/facesengine/detection/opencv-face/opencvfacedetector_p.h M +0 -19 core/libs/facesengine/facedb/facedb.h D +0 -245 core/libs/facesengine/facedb/facedb_lbph.cpp M +1 -6 core/libs/facesengine/facedb/facedb_p.h M +0 -11 core/libs/facesengine/facedb/facedbschemaupdater.cpp M +0 -1 core/libs/facesengine/recognition/README M +0 -8 core/libs/facesengine/recognition/facialrecognition_wrapper_p.cpp M +1 -16 core/libs/facesengine/recognition/facialrecognition_wrapper_p.h M +0 -18 core/libs/facesengine/recognition/facialrecognition_wrapper_setup.cpp M +0 -9 core/libs/facesengine/recognition/facialrecognition_wrapper_training.cpp D +0 -553 core/libs/facesengine/recognition/opencv-lbph/facerec_borrowed.cpp D +0 -187 core/libs/facesengine/recognition/opencv-lbph/facerec_borrowed.h D +0 -230 core/libs/facesengine/recognition/opencv-lbph/lbphfacemodel.cpp D +0 -112 core/libs/facesengine/recognition/opencv-lbph/lbphfacemodel.h D +0 -204 core/libs/facesengine/recognition/opencv-lbph/opencv_face.cpp D +0 -439 core/libs/facesengine/recognition/opencv-lbph/opencv_face.h D +0 -218 core/libs/facesengine/recognition/opencv-lbph/opencvlbphfacerecognizer.cpp D +0 -98 core/libs/facesengine/recognition/opencv-lbph/opencvlbphfacerecognizer.h https://invent.kde.org/graphics/digikam/commit/b6a240f95754eaa188f3aa97c59fdbd9785daa91
Marcel, I cleaned all dead code in face engine, typically all legacy algorithms not based on DNN. I'm not sure if this will have an effect at run time, but at least, now the implementations are more simpler as now more pre-processors puzzle are included in codes. This will definitively simplify the code maintenance in the future. This want mean that, after 10 years of development on face engine, we switch definitively to DNN for detection and recognition (as expected of course)... Gilles
Gilles, thank you for your effort. I will test the code now again. Should I run Digikam with the address sanitizer enabled? In my opinion we could close this huge bug and open new bugs for new problems, is that all right? I followed all of your code changes concerning this bug. In my opinion there is one thing that could increase the readability. You often completed the class definition to match the rule of 3. See https://en.cppreference.com/w/cpp/language/rule_of_three. Why didn't you defined the constructors in a c++11 way? Instead of making it private you could use delete or instead of implement an empty constructor/destructor, default could be used. Or did I miss a technical detail about that?
Git commit 3bcd5d8a28bf10acd0710805b2fa5f8ed2ad42a2 by Gilles Caulier. Committed on 26/09/2020 at 06:46. Pushed by cgilles into branch 'master'. FacesEngine: cleanup stage 3 : move usused FunnelReal class to tests suite as it's not used in core implementation Move data model for FunnelReal algorithm to tests suite. We will deal later if this code can be removed definitively. More cmake scripts cleanup for faces engine. move faces engine tests suite to dedicated subdirectories M +0 -21 core/data/facesengine/CMakeLists.txt D +0 -3 core/data/facesengine/alignment/README D +- -- core/data/facesengine/alignment/flandmark_model.dat.gz M +4 -13 core/libs/facesengine/CMakeLists.txt M +3 -56 core/tests/facesengine/CMakeLists.txt A +33 -0 core/tests/facesengine/alignment/CMakeLists.txt A +1 -0 core/tests/facesengine/alignment/README R +0 -3 core/tests/facesengine/alignment/align.cpp [from: core/tests/facesengine/align.cpp - 099% similarity] R +- -- core/tests/facesengine/alignment/face-funnel.data.tgz [from: core/data/facesengine/alignment/face-funnel.data.tgz - 100% similarity] R +4 -1 core/tests/facesengine/alignment/funnelreal.cpp [from: core/libs/facesengine/alignment/congealing/funnelreal.cpp - 099% similarity] R +0 -0 core/tests/facesengine/alignment/funnelreal.h [from: core/libs/facesengine/alignment/congealing/funnelreal.h - 100% similarity] A +32 -0 core/tests/facesengine/demo/CMakeLists.txt A +27 -0 core/tests/facesengine/preprocess/CMakeLists.txt https://invent.kde.org/graphics/digikam/commit/3bcd5d8a28bf10acd0710805b2fa5f8ed2ad42a2
Hi Marcel, Sorry for my late response but i'm very busy in my office. >Should I run Digikam with the address sanitizer enabled? Both. First without, and in second if it continue to crash, with ASAN. >In my opinion we could close this huge bug and open new bugs for new problems, >is that all right? Not yet. Memory leak is fixed but a crash still here typically with PreviewEngine. Maik work on it. >Why didn't you defined the constructors in a c++11 way? Instead of making it private you could use delete or instead of implement an empty constructor/destructor, default could be used. Or did I miss a technical detail about that? You don't miss anything. Both method are valid, but : 1/ Other classes use C++0x implementation to disable constructors/destructor/operators. We need to still homogeneous everywhere as amount of source code is really huge. If we differ, this will become the hel to maintain. 2/ Don't forget MSVC : this compiler is a shit. C++ support is a pain, sure better with MSVC 2019, but M$ is so far than G++ and Clang to support standard. If you look into MSVC 2019, there is a plugin to use Clang instead native M$ compiler. In fact M$ continue to introduce more and more and more Open Source tools/codes/modules in Windows stuff. What's the next stage ? replace Windows kernel by Linux kernel ??? In fact KDE CI still to use MSVC 2017. So we need to not support totally C++11 yet. 3/ I'm not against whole C++11 standard at all, but some keywords as auto make code opaque and do not help in readbility. Also, why the compiler will automatically cast the target class with auto in a better way that the developper ??? I dislike also the labda method which make the code as a puzzle, as Javascript (excepted for some use where Lambda method can help to improve bianry compatibility but this must not generalized at all). 4/ Clang-tidy is a nice CLI tool to patch automatically all C++0x to C++11. I see an higher activity in Exiv2 project to port all codes to C++11, and clang-tidy is used in background. We will do the same later, but a specific release must be done only for this kind of changes to check for regressions. Gilles
Git commit a2b9c9b54eacdc0b5fc66b4b740d4f5b629e9556 by Gilles Caulier. Committed on 26/09/2020 at 08:03. Pushed by cgilles into branch 'master'. Faces Engine stage 4: remove another legacy unused class OpenCVMatData M +0 -1 core/libs/facesengine/CMakeLists.txt D +0 -82 core/libs/facesengine/common/opencvmatdata.cpp D +0 -65 core/libs/facesengine/common/opencvmatdata.h M +1 -1 core/libs/facesengine/facedb/facedb.h https://invent.kde.org/graphics/digikam/commit/a2b9c9b54eacdc0b5fc66b4b740d4f5b629e9556
Gilles, thank you very much for this detailed explanation. I really appreciate that. So it definitely makes sense as it is :-) @Maik, does it help to upload some resent backtraces and address sanitizer errors or do you have enough information for debuging? Can you reproduce the crashes?
Marcel, Don't forget to install at least asan and lsan devel packages. First one is to check address in uses sanitizer and second one is memory leak sanitizer. Gilles
Created attachment 131985 [details] Again address Sanitizer error on PreviewLoadingTask I tested the face detection feature again. This time with YOLO and clang compiler on 6c5b6ba2f989591aa1913d08ba4a82576aec0721. I get the same error with gcc. This time the error occurs again after a few minutes of scanning. Isn't it in the part that Maik already fixed? @Gilles. I sent you a private message because it does not has anything to do with this bug.
Git commit 85c6c8c4585bcf3e7e779daaf9b17bfa386fab1a by Maik Qualmann. Committed on 28/09/2020 at 19:01. Pushed by mqualmann into branch 'master'. we switch to a QHash for testing M +2 -2 core/libs/threadimageio/fileio/loadingcache.cpp https://invent.kde.org/graphics/digikam/commit/85c6c8c4585bcf3e7e779daaf9b17bfa386fab1a
Created attachment 132002 [details] clang tidy patch for override fixes in theadimageio Generated following this note: https://github.com/KratosMultiphysics/Kratos/wiki/How-to-use-Clang-Tidy-to-automatically-correct-code
Created attachment 132003 [details] clang tidy patch for override fixes in theads Maik, please review the 2 patches generated by cland-tidy modernize C++ python script. Gilles
Feedback about patch: only one override + ~ActionJob() override override; + ~ActionThreadBase() override override; + ~DynamicThread() override override; + ~ThreadManager() override override; + ~WorkerObject() override override; After my knowledge it does the same as before.
Hi Marcel, Yes i seen too the double "override" keyword. Why clang-tidy do that, i don't know. This is why it's very important to review patch created by non human. Gilles
Marcel, Read this (:=))) https://stackoverflow.com/questions/53032915/clang-tidy-inserts-multiple-override-specifiers-when-fixing Gilles
Git commit e73d3b37b565081d6768cc22f75464befa7e4ded by Maik Qualmann. Committed on 29/09/2020 at 17:31. Pushed by mqualmann into branch 'master'. apply the clang tidy patches M +1 -1 core/libs/threadimageio/engine/filereadwritelock.h M +1 -1 core/libs/threadimageio/engine/managedloadsavethread.h M +1 -1 core/libs/threadimageio/engine/sharedloadsavethread.h M +2 -2 core/libs/threadimageio/fileio/loadingcache.h M +1 -1 core/libs/threadimageio/fileio/loadsavetask.h M +10 -10 core/libs/threadimageio/fileio/loadsavethread.h M +1 -1 core/libs/threadimageio/preview/previewloadthread.h M +1 -1 core/libs/threadimageio/preview/previewtask.h M +3 -3 core/libs/threadimageio/thumb/thumbnailloadthread.h M +3 -3 core/libs/threads/actionthreadbase.h M +1 -1 core/libs/threads/dynamicthread.cpp M +1 -1 core/libs/threads/dynamicthread.h M +6 -6 core/libs/threads/parallelworkers.h M +3 -3 core/libs/threads/threadmanager.cpp M +1 -1 core/libs/threads/threadmanager.h M +2 -2 core/libs/threads/workerobject.h https://invent.kde.org/graphics/digikam/commit/e73d3b37b565081d6768cc22f75464befa7e4ded
Git commit e331ff2d3241ce4fec690ddd67a7ffbff1ac587c by Gilles Caulier. Committed on 30/09/2020 at 03:40. Pushed by cgilles into branch 'master'. apply clang tidy override modernize C++11 patch M +4 -4 core/libs/facesengine/detection/opencv-dnn/dnnfacedetectorssd.h M +4 -4 core/libs/facesengine/detection/opencv-dnn/dnnfacedetectoryolo.h M +1 -1 core/libs/facesengine/facedb/facedbbackend.h M +2 -2 core/libs/facesengine/preprocessing/recognition/recognitionpreprocessor.h M +11 -11 core/libs/facesengine/recognition/dataproviders.h M +0 -1 core/libs/facesengine/recognition/facialrecognition_wrapper_p.h M +2 -2 core/libs/facesengine/recognition/opencv-dnn/opencvdnnfacerecognizer_p.h M +3 -2 core/libs/facesengine/recognition/recognitiontrainingprovider.h https://invent.kde.org/graphics/digikam/commit/e331ff2d3241ce4fec690ddd67a7ffbff1ac587c
Git commit edbd8d5d2f401f6c4bbda134749fa092d76b565e by Gilles Caulier. Committed on 30/09/2020 at 03:43. Pushed by cgilles into branch 'master'. apply clang tidy override modernize C++11 patch M +4 -4 core/utilities/facemanagement/database/faceutils.h M +1 -1 core/utilities/facemanagement/items/facegroup.h M +1 -1 core/utilities/facemanagement/items/faceitem.h M +1 -1 core/utilities/facemanagement/threads/facepipeline.h M +1 -1 core/utilities/facemanagement/threads/facepreviewloader.h M +1 -1 core/utilities/facemanagement/threads/parallelpipes.h M +2 -2 core/utilities/facemanagement/threads/scanstatefilter.h M +1 -1 core/utilities/facemanagement/widgets/assignnamewidget.h M +1 -1 core/utilities/facemanagement/widgets/assignnamewidgetstates.h M +1 -1 core/utilities/facemanagement/widgets/facemanagementhelpdlg.h M +3 -3 core/utilities/facemanagement/widgets/facescanwidget.h M +1 -1 core/utilities/facemanagement/workers/databasewriter.h M +1 -1 core/utilities/facemanagement/workers/detectionworker.h M +2 -2 core/utilities/facemanagement/workers/recognitionworker.h M +2 -2 core/utilities/facemanagement/workers/trainerworker.cpp M +2 -2 core/utilities/facemanagement/workers/trainerworker.h https://invent.kde.org/graphics/digikam/commit/edbd8d5d2f401f6c4bbda134749fa092d76b565e
Git commit a296fd327a98a5d5ffbf6690cec4eb5b85de0330 by Maik Qualmann. Committed on 30/09/2020 at 06:04. Pushed by mqualmann into branch 'master'. drastic change to the image cache lock M +0 -1 core/libs/database/utils/scan/scancontroller_start.cpp M +48 -6 core/libs/threadimageio/fileio/loadingcache.cpp M +2 -0 core/libs/threadimageio/fileio/loadingcache.h M +1 -6 core/libs/threadimageio/fileio/loadingcacheinterface.cpp M +71 -72 core/libs/threadimageio/fileio/loadsavetask.cpp M +108 -112 core/libs/threadimageio/preview/previewtask.cpp M +8 -16 core/libs/threadimageio/thumb/thumbnailloadthread.cpp M +4 -8 core/libs/threadimageio/thumb/thumbnailloadthread_p.cpp M +52 -53 core/libs/threadimageio/thumb/thumbnailtask.cpp https://invent.kde.org/graphics/digikam/commit/a296fd327a98a5d5ffbf6690cec4eb5b85de0330
Hi Marcel, it would be good if you could test the last change. Maik
Git commit 84779e8dca5c96503a77a127874a0f13b37894d4 by Maik Qualmann. Committed on 30/09/2020 at 18:10. Pushed by mqualmann into branch 'master'. do not return a pointer from QCache, it can be deleted M +18 -4 core/libs/threadimageio/fileio/loadingcache.cpp M +2 -2 core/libs/threadimageio/fileio/loadingcache.h M +8 -14 core/libs/threadimageio/fileio/loadsavetask.cpp M +6 -12 core/libs/threadimageio/preview/previewtask.cpp M +2 -6 core/libs/threadimageio/thumb/thumbnailtask.cpp https://invent.kde.org/graphics/digikam/commit/84779e8dca5c96503a77a127874a0f13b37894d4
Maik, I tried it with your commit https://invent.kde.org/graphics/digikam/-/commit/a296fd327a98a5d5ffbf6690cec4eb5b85de0330 but it again crashed after a few minutes. Digikam::LoadingCache::notifyNewLoadingProcess(Digikam::LoadingProcess*, Digikam::LoadingDescription const&) /home/marcel/Repos/digikam/core/libs/threadimageio/fileio/loadingcache.cpp:293 Now I'm on https://invent.kde.org/graphics/digikam/-/commit/a0157f8d2bcc6dfb33e74518b546c1553a864ae2 and it works until now (more than 50 minutes). I will keep it running during the night.
No look, it still crashes, at least with asan enabled.
Git commit 92cfeba3a750ba95b4665a4c2e3e8ccfe0dee627 by Maik Qualmann. Committed on 30/09/2020 at 20:44. Pushed by mqualmann into branch 'master'. remove only one listener M +1 -1 core/libs/threadimageio/fileio/loadsavetask.cpp https://invent.kde.org/graphics/digikam/commit/92cfeba3a750ba95b4665a4c2e3e8ccfe0dee627
Git commit 0e2cc57c29f218cb0e9002d0b852aca4c0e25bf2 by Maik Qualmann. Committed on 30/09/2020 at 20:48. Pushed by mqualmann into branch 'master'. use QMap again M +12 -12 core/libs/threadimageio/fileio/loadingcache.cpp https://invent.kde.org/graphics/digikam/commit/0e2cc57c29f218cb0e9002d0b852aca4c0e25bf2
Git commit e2be75c6cf36a6a8252a353679b2a871103d6116 by Maik Qualmann. Committed on 01/10/2020 at 10:39. Pushed by mqualmann into branch 'master'. after revert to the old code, we test this small change M +11 -11 core/libs/threadimageio/fileio/loadingcache.cpp M +1 -1 core/libs/threadimageio/fileio/loadsavetask.cpp https://invent.kde.org/graphics/digikam/commit/e2be75c6cf36a6a8252a353679b2a871103d6116
Maik, the version now looks very promising. It already works for hours without a crash. I saw you reverted most changes (even the typo fix :-) ). In your commit message are you writing "test this small change". What did you change, that we did not test so far?
Created attachment 132054 [details] threadimageio clang-tidy patch about pass by value Maik, clang-tidy provide also fixes about pass by value arguments of methods. This also include to use std::move constructor if necessary. Look well the description here: https://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html Gilles
I'll have to read it through tomorrow, the patch looks strange to me... Maik
Created attachment 132064 [details] asan error in CPGFMemoryStream Maik, the test run for a long time. I original error did not occur. Instead this one was shown after hours. What is now the change? Is it now just the removeOne() function? @Gilles. In my work place we also had discussions about this clang-tidy rule. Technically it can help in a few places to decrease the move effort. But we came up with the conclusion that the readability is lower and its error prune when copying from such a place. Therefore we decided to not to use this rule. Anyway, in my opinion there are other rules far more important for Digikam than this one. On my work place it turned out that a good way to go is: - Activate new rule and auto fix it - Most autofix problems turned out to be compiler errors es well. Fix them. - After applying, look for a compiler flag doing the same (e.g override) and apply it and treat warnings as errors With this way you also find the false positive and ensure that nobody in the future does it wrong again. If you like I could spend some time with that and send you a merge request...
The problem is clear, memory was created with new. It is necessary to increase the memory, now realloc() is used. We should report this to libpgf as a bug. I will patch our internal libpgf. Maik
Git commit 28697b87b1ea439425711eec490df15ab8e089e9 by Maik Qualmann. Committed on 02/10/2020 at 19:24. Pushed by mqualmann into branch 'master'. fix internal libpgf, do not use realloc() when memory was allocated with new M +3 -1 core/libs/pgfutils/libpgf/PGFstream.cpp https://invent.kde.org/graphics/digikam/commit/28697b87b1ea439425711eec490df15ab8e089e9
Git commit 4f16275e2d2c19b59f991d087e98af707daf0ae8 by Maik Qualmann. Committed on 03/10/2020 at 05:20. Pushed by mqualmann into branch 'master'. we can optimize this due to changes in setStatus() M +4 -22 core/libs/threadimageio/fileio/loadsavetask.cpp M +4 -22 core/libs/threadimageio/preview/previewtask.cpp M +4 -20 core/libs/threadimageio/thumb/thumbnailtask.cpp https://invent.kde.org/graphics/digikam/commit/4f16275e2d2c19b59f991d087e98af707daf0ae8
The test runs now for 12 hours without a crash. Good job Maik and Gilles. Thank you for fixing that. I will continue the run to see if it finishes without a crash but I guess we can close this issue.
Maik, About your fix in libpgf, well seen. This is a good candidate for an UPSTREAM report, but i think we can do more. All static analyzers ignore libpgf errors and warnings. There is a rule in .krazy file to drop libpgf reports. As lipgf is a core components used for thumbnails engine, we need to check all major reports in this code. Gilles
Git commit 11bd6030db6497f4ecf3be05daa10c64743e8750 by Gilles Caulier. Committed on 03/10/2020 at 08:02. Pushed by cgilles into branch 'master'. enable static analyzer reports for libpgf to check if major errors are present in code as memory leak M +1 -1 .krazy M +0 -1 project/reports/cppcheck.sh https://invent.kde.org/graphics/digikam/commit/11bd6030db6497f4ecf3be05daa10c64743e8750
Maik, cppcheck and clang-scan reports now parse libpgf. Look the results: https://www.digikam.org/reports/cppcheck/master/ https://www.digikam.org/reports/clang/master/ Gilles
Maik, about the clang-tidy patch for pass by value in constructor, you can forget it. It make the code less clear than before. Gilles
The reported problems in libpgf are almost negligible. And yes, I've never seen anything like the clang-tidy patch for pass by value in a Qt project. Maik
Ok, thank you for the long test. I will close this and other bugs. If you still have a problem, please open a new bug. Maik
I think we can plan to release the 7.2.0 beta1 soon. It's fine for you? Gilles
Yes, I agree. Maik