Review Request 125395: Mutex around usage of m_connectedEvents which may be called from any thread

David Edmundson david at davidedmundson.co.uk
Tue Sep 29 21:55:47 UTC 2015



> On Sept. 29, 2015, 5:15 p.m., Daniel Nicoletti wrote:
> > Why is it better? I found the first less intrusive, simpler and likely faster

The first one wouldn't be faster. We had the queued connection on start() on the timer anyway whereas with patch 2 it's a direct call. So that's the same number of meta object table lookups, and this one doesn't have the the potential for one thread to be locked waiting.

I'm pretty indifferent. I'll push whichever one you prefer.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125395/#review86120
-----------------------------------------------------------


On Sept. 29, 2015, 3:48 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125395/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2015, 3:48 p.m.)
> 
> 
> Review request for Plasma, Daniel Nicoletti and Harald Sitter.
> 
> 
> Repository: print-manager
> 
> 
> Description
> -------
> 
> m_connectedEvents is modified in connectNotify which according to the
> docs will be called from the thread of the caller, not this.
> 
> A mutex around all places that use/modify it should prevent print manager from making plasmashell crash.
> 
> BUG: 345862
> 
> 
> Diffs
> -----
> 
>   libkcups/KCupsConnection.h f61ccb53078766e7f5e96dedec879b52b9083b66 
>   libkcups/KCupsConnection.cpp 482a0bcc9afdee9e0fa131da158988d349dd0da6 
> 
> Diff: https://git.reviewboard.kde.org/r/125395/diff/
> 
> 
> Testing
> -------
> 
> Applet still loads..though I don't have any printers, so can't test too much.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150929/a716ca08/attachment.html>


More information about the Plasma-devel mailing list