Review Request 117209: KDirectoryContentsCounter: do not delete the worker object while it might still be running in another thread

Commit Hook null at kde.org
Thu Apr 3 08:08:03 BST 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117209/#review54914
-----------------------------------------------------------


This review has been submitted with commit 80b7aada681ec24d849f8860405a251ec2771380 by Frank Reininghaus to branch KDE/4.13.

- Commit Hook


On March 31, 2014, 3:23 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117209/
> -----------------------------------------------------------
> 
> (Updated March 31, 2014, 3:23 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Bugs: 332767
>     http://bugs.kde.org/show_bug.cgi?id=332767
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> In Details View, the number of files inside subfolders, which is shown in the "Size" column, is determined in a background thread to prevent that the GUI is blocked while the files are counted. Every view has a KDirectoryContentsCounter object, which communicates with a KDirectoryContentsCounterWorker object, that does the actual work and lives in another thread, via signals and slots.
> 
> Even though this approach to threading was once called "Threading without the headache" ( http://blog.qt.digia.com/blog/2006/12/04/threading-without-the-headache/ ), I still made a stupid mistake in https://git.reviewboard.kde.org/r/115064/ , which lets the worker objects of all views share the same thread (Dolphin 4.12 starts one new thread for every view). If a view is closed, such that the counter and worker objects are not needed any more, the counter will simply delete the worker, even though it might still be busy at the moment. This seems to be the reason for the crash in the bug report.
> 
> Note that there is some code that waits until the worker has finished its work, but it is executed only if the last view is closed, such that the worker thread can be terminated.
> 
> I think that the best solution is to delete the worker object using deleteLater(). This ensures that it is deleted in its own thread, after it has finished its work.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/private/kdirectorycontentscounter.cpp 65afb7c 
> 
> Diff: https://git.reviewboard.kde.org/r/117209/diff/
> 
> 
> Testing
> -------
> 
> I could not really reproduce the crash yet, but I tested splitting the view, opening tabs, and closing some of the new views, and everything seems to work fine.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140403/d054682a/attachment.htm>


More information about the kfm-devel mailing list