----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121134/#review70475 -----------------------------------------------------------
Ship it! kdecore/kernel/kglobal.cpp <https://git.reviewboard.kde.org/r/121134/#comment49283> this comment isn't useful, it's obvious that the use of an atomic int is for thread safety. kdecore/kernel/kglobal.cpp <https://git.reviewboard.kde.org/r/121134/#comment49284> use false instead of 0, and add a "// bool" comment, for the reader to know this is a bool, conceptually, not an int. kdecore/kernel/kglobal.cpp <https://git.reviewboard.kde.org/r/121134/#comment49286> const int prevRefCount = ... kdecore/kernel/kglobal.cpp <https://git.reviewboard.kde.org/r/121134/#comment49285> Would s_allowQuit.load() compile? I forgot the Qt4 API since it changed in Qt5. It would be slightly better imho, for readability. - David Faure On Nov. 16, 2014, 8:22 p.m., René J.V. Bertin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121134/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2014, 8:22 p.m.) > > > Review request for KDE Software on Mac OS X, kdelibs and David Faure. > > > Repository: kdelibs > > > Description > ------- > > I have been experiencing unexpected exits from KDevelop that were not due to > any kind of error in the KDevelop code; it was as if someone told the > application to exit normally. This happens mostly on OS X, but also sometimes > on Linux. > I finally traced this to `KGlobal::deref` reaching a zero reference count and > invoking `QCoreApplication::quit` when called from one of KDevelop's KJob > dtors. There does not appear to be a reference counting mismatch in the code, > so the issue might be due to a race condition in KGlobal::ref/deref. > > This patch introduces thread-safety to KGlobal's reference counting by > turning the simple global `static int s_refCount` into a `static QAtomicInt > s_refCount`. I consider this an important bug fix regardless of whether it > corrects the issue I have with KDevelop. > > > Diffs > ----- > > kdecore/kernel/kglobal.cpp cf003a4 > > Diff: https://git.reviewboard.kde.org/r/121134/diff/ > > > Testing > ------- > > On OS X 10.6.8 only for now. > > NB: Qt itself uses the QAtomicInt's parent class (QBasicAtomicInt) for its > own reference counter used for implicit sharing. This is thus well-tested > code that is unlikely to introduce regressions to a core KDE feature. > > > Thanks, > > René J.V. Bertin > >
