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

[Commit Hook]

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


This review has been submitted with commit a9d7ebbc8be0d03104f3b5c620f8da232cd52857 by Simeon Bird to branch KDE/4.10.

- Commit Hook


On Jan. 12, 2013, 3:58 p.m., Simeon Bird wrote:
> 
> -----------------------------------------------------------
> 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.
> 
> 
> Description
> -------
> 
> 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
> -----
> 
>   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
> -------
> 
> 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/20130115/dd670fd2/attachment.htm>