Review Request 107656: Fix crash due to Qthread being restarted and then deleted

[Simeon Bird]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107656/
-----------------------------------------------------------

(Updated Jan. 12, 2013, 3:58 p.m.)


Review request for Dolphin and Frank Reininghaus.


Summary (updated)
-----------------

Fix crash due to Qthread being restarted and then deleted


Description (updated)
-------

commit c9ff067317155da63393f45d3adf543830fc866b
Author: Simeon Bird <bladud at gmail.com>
Date:   Sat Jan 12 10:43:26 2013 -0500

    A crash occurs if updateItemStates runs between the
    UpdateItemStatesThread finishing and the finished() signal being
    delivered.
    
    In this case, a new thread was not created, because the old thread still
    existed. However, pendingItemStatesUpdate was not set, because the thread was
    not running. Instead, the old thread was restarted.
    
    This meant that the finished() signal from the first run could be delivered
    while the thread was running for a second time, causing the thread to be
    deleted while still running and thus a crash.
    
    Solution: set pendingItemStatesUpdate if the thread is non-null, even if
    it is not running, knowing that slotThreadFinished has not yet run, and
    will call updateItemStates itself.
    
    BUG: 302264
    FIXED-IN: 4.10
    REVIEW: 107656

commit da96efba82abb8848c1288684babc1883ea8cc8d
Author: Simeon Bird <bladud at gmail.com>
Date:   Sat Jan 12 10:37:57 2013 -0500

    The locking around the plugin access in actions doesn't seem to be
    necessary, as actions is only called from the main thread.
    
    Also it wasn't checked consistently; if the lock could not be taken, the
    plugin was accessed anyway.

commit 89711faa7d7718ed35ff77fe5d2df3e502102ecf
Author: Simeon Bird <bladud at gmail.com>
Date:   Sat Jan 12 10:37:26 2013 -0500

    We don't need the mutex guarding m_itemStates in the
    UpdateItemStatesThread, because m_itemStates is only accessed by the
    when the thread is done, and set before the thread starts.
    
    Also combine the setData function with the constructor.


This addresses bug 302264.
    http://bugs.kde.org/show_bug.cgi?id=302264


Diffs (updated)
-----

  dolphin/src/views/versioncontrol/updateitemstatesthread.h f0f91d7 
  dolphin/src/views/versioncontrol/updateitemstatesthread.cpp e07d72c 
  dolphin/src/views/versioncontrol/versioncontrolobserver.cpp 42e00de 

Diff: http://git.reviewboard.kde.org/r/107656/diff/


Testing (updated)
-------

The crash can be made very easy to reproduce by shortening the verificationTimer in VersionControlObserver to 50msec. 
With this patch, the crash goes away.


Thanks,

Simeon Bird

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130112/b040d3ea/attachment.htm>