[KDE/Mac] Review Request 121134: make KGlobal reference counting threadsafe

[René J.V. Bertin]

> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20141116/7ffb248d/attachment.html>