D7115: Simplify search synchronization

Daniel Vrátil noreply at phabricator.kde.org
Fri Aug 4 10:02:56 BST 2017


dvratil requested changes to this revision.
dvratil added a comment.
This revision now requires changes to proceed.


  `SearchManager::updateSearch()` can be called from arbitrary thread, but it's important that `SearchManager::updateSearchImpl()` is executed in `SearchManager`'s thread, your change break this. We should probably add `Q_ASSERT(QThread::currentThread() == thread());` at the beginning of `updateSearchImpl()` and properly documented the requirement (it's not very obvious from the comment in the header).
  
  Also good point with the FIXME, probably need to do something like storing a `QWaitCondition`s in the `mUpdatingCollections` vector so that others can wait and be notified when the search is finished.

INLINE COMMENTS

> searchmanager.cpp:273
> -    QMetaObject::invokeMethod(this, "updateSearchImpl",
> -                              Qt::QueuedConnection,
> -                              Q_ARG(Collection, collection),

Replace `Qt::QueuedConnection` by `Qt::BlockingQueuedConnection` and you can still remove the `QSemaphore`. No idea why I did not use that in the first place... :-)

REPOSITORY
  R165 Akonadi

REVISION DETAIL
  https://phabricator.kde.org/D7115

To: dkurz, #kde_pim, dvratil
Cc: dvratil, dvasin, winterz, vkrause, mlaurent, knauss
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20170804/804d3e4a/attachment.html>


More information about the kde-pim mailing list