Bug 345954 - Recursive calls made into ProgressManager::slotTransactionCompletedDeferred [patch]
Summary: Recursive calls made into ProgressManager::slotTransactionCompletedDeferred [...
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: ProgressManager-Engine (show other bugs)
Version: 4.9.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-07 14:27 UTC by Kristian Karl
Modified: 2015-04-10 19:36 UTC (History)
2 users (show)

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


Attachments
Proposed patch fixing the bug (1.09 KB, patch)
2015-04-07 14:29 UTC, Kristian Karl
Details
A proposed patch (1.97 KB, patch)
2015-04-08 15:14 UTC, Kristian Karl
Details
progressmanager.patch (1.03 KB, patch)
2015-04-08 20:17 UTC, Maik Qualmann
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kristian Karl 2015-04-07 14:27:09 UTC
When removing thousands of face tags from thousands of images, I noticed that the process of removal of face tags took way to long time. 

1) I have a test set of 3152 images. Those images have 1738 face tags
2) In the "People view", I select all  (1738) face tags.
3) I click on the "Reject if this is not a face" button (Upper right corner of a face icon)

Expected results
==============
All selected face tags are removed.

Actual results
===========
The process of removing the face tags, begun, but the process never ended. I seams as it hung somewhere.


I did some troubleshooting (and with some good help from a friend) I found what I believe is a bug, causing this 'hanging'.

Reproducible: Always
Comment 1 Kristian Karl 2015-04-07 14:29:10 UTC
Created attachment 91925 [details]
Proposed patch fixing the bug
Comment 2 Maik Qualmann 2015-04-07 19:34:17 UTC
Git commit 685e0764bb3b771afbc465e246307e278a12e18d by Maik Qualmann.
Committed on 07/04/2015 at 19:24.
Pushed by mqualmann into branch 'master'.

apply patch #91925 from Kristian Karl to corrects the signal connection in the ProgressManager
FIXED-IN: 4.10.0

M  +2    -1    NEWS
M  +1    -1    libs/progressmanager/progressmanager.cpp

http://commits.kde.org/digikam/685e0764bb3b771afbc465e246307e278a12e18d
Comment 3 caulier.gilles 2015-04-07 20:38:32 UTC
Git commit c518182aa399cd1f0c456bf191d88d200ae4f5a0 by Gilles Caulier.
Committed on 03/04/2015 at 09:20.
Pushed by cgilles into branch 'frameworks'.

backport commit #685e0764bb3b771afbc465e246307e278a12e18d from git/master to frameworks branch

M  +1    -1    libs/progressmanager/progressmanager.cpp

http://commits.kde.org/digikam/c518182aa399cd1f0c456bf191d88d200ae4f5a0
Comment 4 Maik Qualmann 2015-04-07 21:36:54 UTC
Hi Kristian,
I now get this message in the console when you start digiKam:

Object::connect: No such signal Digikam::NewItemsFinder::completeTransactionDeferred(ProgressItem*)

The completeTransactionDeferred(ProgressItem*) signal do not exist in ProgressItem implementation.

Maik
Comment 5 Kristian Karl 2015-04-08 06:21:40 UTC
Hi Mail,
Good catch Maik!
I noticed that the progress items never gets removed after the face tag removal task is done. It's most likely connected to your observation. I'm working on a fix.

Kristian
Comment 6 Kristian Karl 2015-04-08 15:14:43 UTC
Created attachment 91947 [details]
A proposed patch

The first patch did not fix this bug completely. It introduced a new bug, where the statusbarprogresswidget was not able to signal that it was done. Thus the status progress bar was never removed when it reached 100%

This patch fixes that problem. I tested it for a couple of use cases, where progress bar is used, like
* Removing face tags
* Fingerprinting images
* Re-reading metadata from files

However, I suggest that this patch is code reviewed by someone that knows the progress manager and status bar items/widgets well.
Comment 7 Maik Qualmann 2015-04-08 20:17:44 UTC
Created attachment 91955 [details]
progressmanager.patch

This patch would be an alternative solution to the issue.

Maik
Comment 8 Kristian Karl 2015-04-09 07:45:10 UTC
I tested your patch, which looks very good,  and I think it works nicely. 
Also, your patch is better than my last patch, since it much less intrusive, and thus less likely to introduce regression.
Comment 9 Maik Qualmann 2015-04-09 20:26:59 UTC
Git commit b3d67ced05fb99561d69008e7c9e934d186d4aa3 by Maik Qualmann.
Committed on 09/04/2015 at 20:23.
Pushed by mqualmann into branch 'master'.

apply patch #91955 to corrects the signal connection in the ProgressManager

M  +3    -3    libs/progressmanager/progressmanager.cpp

http://commits.kde.org/digikam/b3d67ced05fb99561d69008e7c9e934d186d4aa3
Comment 10 caulier.gilles 2015-04-10 19:36:08 UTC
Git commit 6d5abbe5d89077b97d91fc8086519f497b85619f by Gilles Caulier.
Committed on 10/04/2015 at 16:33.
Pushed by cgilles into branch 'frameworks'.

backport commit #b3d67ced05fb99561d69008e7c9e934d186d4aa3 from git/master to frameworks branch

M  +3    -3    libs/progressmanager/progressmanager.cpp

http://commits.kde.org/digikam/6d5abbe5d89077b97d91fc8086519f497b85619f