KTextEditor smartLock assertion line is wrong

dukju ahn dukjuahn at gmail.com
Sun May 6 14:51:43 BST 2007


Hi core guys.

Inside the KDevelop, we create a separate thread to parse .cpp file.
That thread should lock smartMutex in smartInterfaces. But the
assertion " bool KateDocument::isSmartLocked() " fails.

Interestingly I've found that " bool KateDocument::isSmartLocked() "
is buggy. The smartMutex is initialized with Recursive mode at
its constructor. And Recursive mutex allows multiples of lock() and
tryLock(). So although .cpp parser thread correctly locked mutex,
inside isSmartLocked() tryLock() gains lock and return value becomes
false. (Please see the implementation of
bool KateDocument::isSmartLocked() const )

The implementation of isSmartLocked() seems to be bought in
Qt Documentation at bool QMutex::locked(). But that documentation
says that

" Returns true if the mutex is locked by ANOTHER thread "

It seems that the troll developers overlooked the case of recursive
locking.

2007/5/5, Jakob Petsovits <jpetso at gmx.at>:
> On Saturday, 5. May 2007, Andreas Pakulat wrote:
> > Hi,
> >
> > somehow the duchain seems to be broken, opening a .cpp file in KDevelop
> > asserts in kateviewinternal:3180. According to Christoph Cullman its
> > probably wrong locking and indeed the assert checks that isSmartLocked()
> > is set.
>
> Ah right. This was the crash I asked you about (which did not happen on your
> system at that time), approximately two weeks ago.
>
> It's caused by a QMutexLocker which does not lock even if it really should,
> in lib/editor/editorintegrator.cpp:190 and the following lines:
>
> Range* newRange = data()->topRanges[currentUrl()][type] =
>         createRange(currentDocument()->documentRange());
> if (SmartInterface* iface = smart()) {
>     QMutexLocker lock(iface->smartMutex());
>     Q_ASSERT(newRange->isSmartRange());
>     iface->addHighlightToDocument( newRange->toSmartRange(), false );
>     newRange->toSmartRange()->addWatcher(data());
> }
>
> The line
>     iface->addHighlightToDocument( newRange->toSmartRange(), false );
> crashes, because the QMutexLocker two lines above DOES NOT LOCK the mutex.
> Check it for yourself by adding the debug lines
>
> bool smartLocked = true;
> if (iface->smartMutex()->tryLock()) {
>     iface->smartMutex()->unlock();
>     smartLocked = false;
> }
>
> directly after the QMutexLocker creation.
> (That's the code that Katepart uses to check if the mutex is locked.)

I've found the reason of error. The reason was the kdelibs/ktexteditor
The smartMutex is initialized with QMutex::Recursive mode. In case of
recursive mutex, one single thread can gain a lock again and again.
So in above code, the tryLock() can always gain a lock, and the return
value was false.

In short, the assert code which ktexteditor used was a bug. Only after
the mutex was initted with Non-Recursive more, or we change the
Q_ASSERT code in kdelibs, the error cannot be solved.




More information about the kde-core-devel mailing list