> On Nov. 16, 2014, 3:58 p.m., Milian Wolff wrote: > > as I said before: Did you check whether that is actually the case? Did you > > add some assertions to see what other thread is calling this code? > > > > I'm not sure whether this is supposed to be threadsafe or not. If it must > > be threadsafe, you'll also need to make s_allowQuit threadsafe > > (QAtomicBool).
I have indeed looked at this aspect, but not exhaustively and haven't found evidence of cross-thread reference counting. Yet. It's unclear what you mean with "that" (the thing actually the case). It "that" refers to the quitting issue, I can of course never prove that it is due to a reference counting race condition because the issue isn't reproducible reliably. I can only invalidate the idea that it is due to such a race condition, whenever it happens again with properly protected reference counting. I think we have to leave it up to the kdelibs maintainers/authors to decide whether or not this has to be thread safe. The doxygen documentation for KJob does not mention anything about thread safety. It does however note that *KJob and its subclasses is meant to be used in a fire-and-forget way. It's deleting itself when it has finished using deleteLater() so the job instance disappears after the next event loop run.* . It is not said that a KJob cannot use a KJob itself, which could lead to concurrent reference (de)counting, nor whether a KJob instance is guaranteed to be deleted in the same thread it was created, and I think that a *concurrent processing* class that does not explicitly mention it does thread unsafe things can be assumed to be thread safe. There's no QAtomicBool, btw. - René J.V. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121134/#review70441 ----------------------------------------------------------- On Nov. 16, 2014, 3:12 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, 3:12 p.m.) > > > Review request for KDE Software on Mac OS X and kdelibs. > > > 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. > > > Thanks, > > René J.V. Bertin > >
