QFileInfoGatherer: fix race conditions pt.1: abort
authorMarc Mutz <marc.mutz@kdab.com>
Tue, 7 Aug 2012 17:17:09 +0000 (19:17 +0200)
committerThe Qt Project <gerrit-noreply@qt-project.org>
Wed, 19 Sep 2012 17:54:44 +0000 (19:54 +0200)
commit6d9d2c4fa874953ddfa14df8189cd93bc1147952
tree97460eceac5c682d1ccea8c9a4f2de972a94fe44
parent63bc298fb336e285c5abf5946eb790b4ba7df372
QFileInfoGatherer: fix race conditions pt.1: abort

Fix a race on the 'abort' variable. While there was a mutex lock around
the code that sets the variable in ~QFileInfoGatherer, there was no
protection in getFileInfos(), where it is read:

   // T:this->thread()  // T:*this
                        // in getFileInfos(), after last mutex.unlock()
   mutex.lock();
   abort = true;        while (!abort...
   // ...               // ...

Fix by making 'abort' an atomic. This means that we can drop the mutex
locker in the destructor, too. We still mostly access 'abort' under
protection of the mutex, because we need to protect other variables that
just happen to be accessed together with 'abort', but we avoid the mutex
lock/unlock on each iteration of the while loop in getFileInfos().

Also cleaned up the logic in run():

- by using the canonical form of condition.wait() (in a loop that
  checks the condition), we can ensure that !path.isEmpty() and avoid
  having to use the updateFiles boolean.
- by checking for abort.load() after we return from
  condition.wait(), we minimise the waiting time for thread
  exit.
- by using different local names, we avoid having to this->qualify members.

Also changed one condition.wakeOne() to wakeAll() for consistency
with fetchExtendedInformation().

Change-Id: If35f338fe774546616ec287c1c37e2c32ed05f1a
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: David Faure <faure@kde.org>
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
src/widgets/dialogs/qfileinfogatherer.cpp
src/widgets/dialogs/qfileinfogatherer_p.h