> 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).
>
> René J.V. Bertin wrote:
> 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.
>
> Milian Wolff wrote:
> As I told you before in other patches you send in: Please make sure you
> find the actual reason before changing other code in the hope of it improving
> things. If there is no hard evidence that your bug manifests itself because
> of a race in KGlobal::ref/deref, then don't change it. Adding an assertion to
> the code that its only accessed from the mainthread is done easily and should
> be hit reliably (contrary to a race condition or lock contention that Thomas
> proposes).
>
> And also note that a KJob is not neccessarily implemented using threads.
> You can also use e.g. QIODevice or external processes internally to get the
> desired behavior.
>
> René J.V. Bertin wrote:
> KJobs not using threads to implement concurrency will not cause race
> conditions in the reference counting; either they are in separate processes
> or they live in the same thread.
>
> Let me repeat once more what I also wrote in reply to Thomas below: a
> feature (like reference counting) of a framework that is frequently used in
> multithreaded applications can be expected to be thread safe. That's why my
> original description states that I consider the fact that it isn't an
> important bug regardless of whether it's the cause of my issue. Regrettably I
> am a scientist trained to give background anytime I propose something, I keep
> forgetting that there are contexts where that actually turns out
> counterproductive :(
>
> You suggest to use an assert or other detection of doing reference
> counting off the main thread, and doing so imply that reference counting
> should apparently happen only on that thread. There is no evidence that that
> is any more valid an assumption than that it should be thread safe. But I can
> do it ... only it won't tell us if multiple threads try to access the counter
> at the same time.
>
> NB: my initial proposition used about the cheapest way to get interlocked
> counting I could think of, to introduce as little side-effects as possible
> for applications that are not concerned by possible race conditions. I don't
> believe there can be any other side-effect than additional cycles (though by
> using QAtomicInt we have to assume that it is implemented correctly).
A little extract, seconds after relaunching KDevelop with
```C++
void KGlobal::ref()
{
s_refCount.fetchAndAddOrdered(1);
if ( QThread::currentThread() != qApp->thread() ) {
qWarning() << "KGlobal::ref() called off the main thread" <<
kBacktrace();
}
//kDebug() << "KGlobal::ref() : refCount = " << int(s_refCount);
}
void KGlobal::deref()
{
int prevRefCount = s_refCount.fetchAndAddOrdered(-1);
if ( QThread::currentThread() != qApp->thread() ) {
qWarning() << "KGlobal::ref() called off the main thread" <<
kBacktrace();
}
//kDebug() << "KGlobal::deref() : refCount = " << int(s_refCount);
if (prevRefCount <= 1 && int(s_allowQuit)) {
QCoreApplication::instance()->quit();
}
}
```
```
kdevelop(68673)/kdevelop (cpp support) Cpp::instantiateDeclarationAndContext:
Resolved bad base-class "QtSharedPointer::ExternalRefCount< T >"
"QtSharedPointer::ExternalRefCount< T >"
kdevelop(68673)/kdevelop (cpp support) Cpp::instantiateDeclarationAndContext:
Resolved bad base-class "QtSharedPointer::ExternalRefCount< T >"
"QtSharedPointer::ExternalRefCount< T >"
kdevelop(68673)/kdevelop (cpp support) Cpp::instantiateDeclarationAndContext:
Resolved bad base-class "QtSharedPointer::ExternalRefCount< T >"
"QtSharedPointer::ExternalRefCount< T >"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
kdevelop(68673)/kdevelop (cpp support) Cpp::instantiateDeclarationAndContext:
Resolved bad base-class "_List_base< _Tp, _Alloc >" "_List_base< _Tp, _Alloc >"
kdevelop(68673)/kdevelop (cpp support) Cpp::instantiateDeclarationAndContext:
Resolved bad base-class "QVector< T >" "QVector< T >"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
KGlobal::ref() called off the main thread "[
0: 0 libkdecore.5.dylib 0x00000001000ba123
_Z14kRealBacktracei + 67
1: 1 libkdecore.5.dylib 0x0000000100100d89 _ZN7KGlobal3refEv
+ 169
2: 2 libkdecore.5.dylib 0x00000001000e49f1
_ZN4KJobC2EP7QObject + 65
3: 3 kdevcmakemanager.so 0x000000012f85f3af
_ZN21CMakeCommitChangesJobC1ERKN8KDevelop4PathEP12CMakeManagerPNS0_8IProjectE +
31
]
"
```
- 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, 4:53 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, 4:53 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
>
>
