PauseableTimer issue + suggested fix

[David Faure]

Hi Dan,

I was looking at why akonadiserver was outputting lots of warnings like
Akonadi::Server::PauseableTimer::pause: Cannot pause an inactive timer

and I think I found why: it comes from the commit attached.

The queued connection for the calls to pause and resume means that
the checks in inhibit(), like mScheduler->isActive() && !mScheduler->isPaused() are not enough anymore since this change.
A queued call to pause() might have been done and not handled yet, so this will create a second queued call,
which will lead to the warning.
In fact, if this code is indeed called from a different thread than the timer, like the commit log says,
then there is a race condition on mScheduler's data members, read in the secondary thread (in inhibit())
and written in the main thread (where PauseableTimer lives, if the commit log is correct).

Wouldn't it make sense to just always make the queued calls, from inhibit(),
and to move the checks for whether we have anything to change, inside pause and resume,
i.e. early-return rather than warning, if there's nothing to do?
This would fix the warning (by definition ;) and it would fix the race because
the thread calling inhibit() wouldn't try to read anything from the thread where the timer lives.


PS: I was looking at debug output because I'm experiencing much bigger problems with akonadi master,
kolab resource not syncing the inbox sometimes, and an akonadiserver assert in QDataStream due to null QIODevice,
but I need more data to give more precise bug reports and/or patches about this.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 13079b3.diff
Type: text/x-patch
Size: 1962 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20161215/536f4455/attachment.bin>