Review Request 112277: Fix deadlock by avoid calling Qfont::QFont() in KFontSettingsData

[Kevin Ottens]

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



staging/frameworkintegration/src/platformtheme/kfontsettingsdata.cpp
<http://git.reviewboard.kde.org/r/112277/#comment28525>

    On second thought. Maybe add a comment here on why it's done in two steps this way.


- Kevin Ottens


On Aug. 25, 2013, 8:38 p.m., Àlex Fiestas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112277/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2013, 8:38 p.m.)
> 
> 
> Review request for KDE Frameworks and Kevin Ottens.
> 
> 
> Description
> -------
> 
> While writing tests for KFontSettingsData I found this deadlock:
> http://paste.kde.org/p1fada663/
> 
> Shorter version of the deadlock by calling QFont::QFont in KFontSettingsData::font()
> http://paste.kde.org/pda7fc276/
> 
> Basically:
> QApplicationPrivate::initializeWidgetFontHash
> KFontSettingsData::font
> QVariant magic, which calls QFont::QFont (http://qt.gitorious.org/qt/qtbase/blobs/stable/src/gui/text/qfont.cpp#line677)
> QFont::QFont
> QGuiApplication::font
> KFontSettingsData::font
> QFont::QFont
> QGuiApplication::font
> And lock takes place
> 
> So the patch simply avoids calling QFont::QFont by reusing the QFont we already have (initialized with the default values) and using QFont::fromString in case we have data to set to it.
> 
> To reproduce simply set fonts in kdeglobals:
> [General]
> desktopFont=Ubuntu,9,-1,5,50,0,0,0,0,0
> fixed=Ubuntu Mono,10,-1,5,50,0,0,0,0,0
> font=Ubuntu,9,-1,5,50,0,0,0,0,0
> menuFont=Ubuntu,9,-1,5,50,0,0,0,0,0
> smallestReadableFont=Ubuntu,8,-1,5,50,0,0,0,0,0
> taskbarFont=Ubuntu,9,-1,5,50,0,0,0,0,0
> toolBarFont=Ubuntu,8,-1,5,50,0,0,0,0,0
> 
> and then execute QApplication.
> 
> 
> Diffs
> -----
> 
>   staging/frameworkintegration/src/platformtheme/kfontsettingsdata.cpp 70c1d26 
> 
> Diff: http://git.reviewboard.kde.org/r/112277/diff/
> 
> 
> Testing
> -------
> 
> Tested in different app's using qpa plus the kfontsettingsdata test I'm writing.
> 
> 
> Thanks,
> 
> Àlex Fiestas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130826/22a6887a/attachment.html>