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
