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

Frank Reininghaus frank78ac at googlemail.com
Mon Mar 31 16:23:25 BST 2014


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

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/6094c0a1/attachment.htm>


More information about the kfm-devel mailing list