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

[Emmanuel Pescosta]

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

Ship it!


Looks good and thanks for the detailed description!

I can't reproduce the bug, so I'm not able to test it.

- Emmanuel Pescosta


On March 31, 2014, 5: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, 5: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/20140331/940ad9fe/attachment.htm>