D24752: WIP: Use a separate worker thread to run the actual deletion of files

[David Faure]

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


  `slotReport` isn't the only problem, all of the subjob handling etc. seems very racy to me.
  
  I suggest to start from scratch and write the smallest possible worker, whose only job is to delete a list of files, and report progress/completion via signal/slots or invokeMethod.

INLINE COMMENTS

> deletejob.cpp:161
> +    connect(d_func()->m_reportTimer, &QTimer::timeout, this, [this] () {
> +        d_func()->slotReport();
> +    });

This calls slotReport in the main thread, where it will read many values from the worker. Those values are written by the worker, from a secondary thread. This is the very definition of a race condition.

I thought the plan (in the task's pseudo-code) was to move into a worker, as little data as possible. And to never access that data from two concurrent threads.

In this patch, almost all of the members are moved to the worker, which creates race conditions all around.

I'm updating my "thread sanitizer" build of Qt and KIO to prove it, but I'm pretty sure :-)

> deletejob.cpp:180
> +    ownThread = QThread::currentThread();
> +    m_ioThread = new QThread();
> +    ioWorker.moveToThread(m_ioThread);

Where is the corresponding `delete` statement?

REPOSITORY
  R241 KIO

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

To: meven, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191020/a0dd1425/attachment-0001.html>