> On Jan. 27, 2015, 8:55 p.m., Matthew Dawson wrote:
> > Unforunately, this cause test system failures in the the
> > kconfigskeletontest test suite. I'm not sure why this should create issues
> > there.
> >
> > However, I have a partial solution that avoids creating a full
> > KSharedConfig. Instead, only globalData needs to be called in KConfig to
> > ensure the object is created as soon as possible (without needing a
> > QCoreApplication). This should avoid having any other globals created
> > before.
> >
> > However, this causes a crash later. The root of the issue is in the
> > KConfigPrivate constructor, line 98. Creating the QLocale after
> > QCoreApplication is gone is apparently broken as well. I'm not sure how to
> > solve that, maybe cache the value we need from QLocale, similar to caching
> > the arguments?
> >
> > Also, I think the copied over KConfig test case really needs to match the
> > KSharedConfig test, as using KSharedConfig can mask the problem since
> > KSharedConfig's pointer is cached, avoiding the KConfig constructor.
>
> David Faure wrote:
> Oh. Indeed. I can explain the test failure: that test was removing the
> config file and then creating a KConfigSkeleton around it (which was supposed
> to be the instanciation of the KSharedConfig, starting from "no file"). With
> my change, the KSharedConfig already exists before that, we delete the file
> underneath, but it keeps existing, so the KConfigSkeleton also uses the data
> from that now-deleted file. It's more or less like refcounting - we kept an
> old object alive by creating it upfront.
>
> You're right, we could just store the args instead, for a more minimal
> change.
> But I tried it and I hit the same QLocale issue as you did. Argh.
> We're really doing too much in that global object dtor. Wrapping up is
> ok, creating new stuff is not.
>
> I tried working around the issue by keeping a KSharedConfig::Ptr data
> member in the Tester class - which sounds like an extremely good idea for
> apps to do, in order to reuse it rather than recreate and reparse at shutdown
> time. This takes care of that issue, but another one then shows up:
> ~Tester -> ~KSharedConfig -> KConfig::sync -> QLockFile::lock ->
> "QApplication::qAppName: Please instantiate the QApplication object first".
> We didn't hit that in kdelibs4 either because I wrote QLockFile for Qt5,
> and KLockFile didn't call qAppName, it used a refcounting KComponentData.
> I guess I could add an if (qApp) in QLockFile, but we're really going
> into fragile territories. We're definitely not just fixing a regression
> anymore since kdelibs4 asserted with this testcase.
>
> The KSharedConfig testcase doesn't avoid the KConfig constructor, the way
> it's written. The caching is refcounting, it only actually caches if there's
> something storing a Ptr somewhere (hence my suggestion to actually do that,
> because apps should really do that, especially since they don't have
> KComponentData doing that for them anymore). I'm fine with any change you
> want to see in the KConfig testcase though, but let's sort out the main
> testcase first.
>
> I'm currently thinking Revision 2 of this patch was the best, it fixed
> the regression compared to kdelibs4 without introducing new issues.
>
> Matthew Dawson wrote:
> I'm thinking using KConfig when a QCoreApplication is not present should
> just not be supported. At the very least, the new issue in the save path
> means you can't actually save anything when the global destructors are
> running, which makes using KConfig then pointless. If we can get QLockFile
> to work correctly in this case, we could revisit this.
>
> For now, I think the best plan is going back to revision 2 (as you
> suggested) + a comment on KConfig/KSharedConfig warning about using them
> without a valid QCoreApplication is not supported. Does that sound good to
> you?
"using without a valid QCoreApplication is not supported" .. funny because this
commit is especially about making it work, at least in some cases.
Using anything from Qt before creating a QCoreApplication is a bad idea anyway
(e.g. locale8Bit not set up) - but I'm happy to repeat that in the
K{Shared}Config documentation.
On the other hand, using KConfig after QCoreApplication is deleted (e.g. in a
global object dtor) does seem to work, says the unittest.
Only KSharedConfig is an issue, depending on the order of construction. So I
added a comment in the KSharedConfig class docu.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122232/#review74858
-----------------------------------------------------------
On Jan. 27, 2015, 8:10 a.m., David Faure wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122232/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2015, 8:10 a.m.)
>
>
> Review request for KDE Frameworks and Matthew Dawson.
>
>
> Repository: kconfig
>
>
> Description
> -------
>
> kconfig_in_global_object.cpp comes from kdelibs4support
> (after porting to Q_GLOBAL_STATIC)
>
> ksharedconfig_in_global_object.cpp is new (but works with kdelibs4)
> and reproduces Albert's KgDifficulty testcase.
>
>
> Diffs
> -----
>
> autotests/kconfig_in_global_object.cpp PRE-CREATION
> autotests/ksharedconfig_in_global_object.cpp PRE-CREATION
> src/core/kconfig.cpp 782e9714521234a3e3d8f3a788967e5c9a40f38a
> src/core/ksharedconfig.cpp e059b87a1cc1df50693a668ef791e7a61050ef88
> autotests/CMakeLists.txt b91f754b705fc87bb8b729bea72fbb5f7d427ace
>
> Diff: https://git.reviewboard.kde.org/r/122232/diff/
>
>
> Testing
> -------
>
> Both tests pass and the QCoreApplication::arguments warning (because called
> after qApp is destroyed) is gone.
>
>
> Thanks,
>
> David Faure
>
>
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
