Bug 304796

Thanks for the bug report.

First, however, you should never ever do this - 
 
> Nepomuk2::Resource res(QUrl("http://www.semanticdesktop.org/ontologies/2007/01/19/nie#title"));
> res.property(QUrl("http://www.w3.org/TR/ol-ref/#range-def"));

The resources should either be initialized by their resource uri or by the nie:url, which would be either something of the form '/home/user/asfadfa' or akonadi:?item=342. You're trying to initialize a resource with a property name.

That being said, we still shouldn't lead to an infinite loop and crash. We should gracefully output an error message, and inform you about evil doings.

I think ideally, we should not be using Resource inside the ResourceWatcher which is used in Resource. It leads to unnecessary complications like the one you have mentioned. Perhaps it would be better to have a special resource watcher just for the Resource class.

For now I'm adding the count function to the ResourceWatcher.

I've created a review request - https://git.reviewboard.kde.org/r/105927/

Could you please test it out?

Created attachment 73051 [details]
Patch to remove calls to Resource::fromResourceUri() in ResourceData::resetAll()

There was still another call to Resource::fromResourceUri() in ResourceData::resetAll(). The above patch removes that too by introducing an overloaded method ResourceWatcher::removeResource(const QUrl& uri).

Now the infinite loop is avoided. There's just a failed assert on exit:

ASSERT: "d->m_urlKickOff.isEmpty()" in file /home/tuukka/code/nepomuk-bug/nepomuk-core/libnepomukcore/resource/resourcemanager.cpp, line 219

> First, however, you should never ever do this - 
>  
> > Nepomuk2::Resource res(QUrl("http://www.semanticdesktop.org/ontologies/2007/01/19/nie#title"));
> > res.property(QUrl("http://www.w3.org/TR/ol-ref/#range-def"));

Then how to get the range of a property?

(In reply to comment #5)
> 
> > First, however, you should never ever do this - 
> >  
> > > Nepomuk2::Resource res(QUrl("http://www.semanticdesktop.org/ontologies/2007/01/19/nie#title"));
> > > res.property(QUrl("http://www.w3.org/TR/ol-ref/#range-def"));
> 
> Then how to get the range of a property?

#include <Nepomuk2/Types/Property>
#include <Nepomuk2/Types/Class>
#include <Nepomuk2/Vocabulary/NIE>

using namespace Nepomuk2::Vocabulary;

Nepomuk2::Types::Property nieTitle( NIE::title() );
kDebug() << nieTitle.range() << nieTitle.literalRangeType();

------------

http://api.kde.org/4.x-api/kdelibs-apidocs/nepomuk-core/html/classNepomuk2_1_1Types_1_1Property.html

(In reply to comment #3)
> Created attachment 73051 [details]
> Patch to remove calls to Resource::fromResourceUri() in
> ResourceData::resetAll()

Unfortunately, I cannot commit this, cause it changes the signature of a method. That breaks compatibility with previous versions.

(In reply to comment #7)
> (In reply to comment #3)
> > Created attachment 73051 [details]
> > Patch to remove calls to Resource::fromResourceUri() in
> > ResourceData::resetAll()
> 
> Unfortunately, I cannot commit this, cause it changes the signature of a
> method. That breaks compatibility with previous versions.

Ok. Well, I'm sure you'll come up with some long term solution.

Git commit 7bef7c53d3b9a971c203ed4391bf19ac79f381f5 by Vishesh Handa.
Committed on 08/08/2012 at 16:08.
Pushed by vhanda into branch 'KDE/4.9'.

ResourceWatcher: add *count() methods for resources, properties and types

These count methods are faster than returning the full list and then
counting the number of items in the list.

This also fixes an infinite loop where a temporary Resource is created
when querying for the number of resources being watched. The temporary
Resource is deleted, which results in ResourceData::resetAll being
callled which again called ResourceWatcher::resources(), and the cycle
countinues.
REVIEW: 105927

M  +16   -0    libnepomukcore/datamanagement/resourcewatcher.cpp
M  +27   -0    libnepomukcore/datamanagement/resourcewatcher.h
M  +1    -1    libnepomukcore/resource/resourcedata.cpp

http://commits.kde.org/nepomuk-core/7bef7c53d3b9a971c203ed4391bf19ac79f381f5

(In reply to comment #9)
> Git commit 7bef7c53d3b9a971c203ed4391bf19ac79f381f5 by Vishesh Handa.
> Committed on 08/08/2012 at 16:08.
> Pushed by vhanda into branch 'KDE/4.9'.

Maybe I didn't make it clear that the infinite loop persists as long as the second call to Resource::fromResourceUri is there:

QMetaObject::invokeMethod(m_rm->m_watcher, "removeResource", Qt::AutoConnection, Q_ARG(Nepomuk2::Resource, Resource::fromResourceUri(m_uri)));

So I wouldn't call this bug fixed yet.

(In reply to comment #10)
> (In reply to comment #9)
> > Git commit 7bef7c53d3b9a971c203ed4391bf19ac79f381f5 by Vishesh Handa.
> > Committed on 08/08/2012 at 16:08.
> > Pushed by vhanda into branch 'KDE/4.9'.
> 
> Maybe I didn't make it clear that the infinite loop persists as long as the
> second call to Resource::fromResourceUri is there:
> 
> QMetaObject::invokeMethod(m_rm->m_watcher, "removeResource",
> Qt::AutoConnection, Q_ARG(Nepomuk2::Resource,
> Resource::fromResourceUri(m_uri)));
> 
> So I wouldn't call this bug fixed yet.

I know. I should have reopened this :)

I managed to trigger this bug in a quite general situation:

{
Nepomuk2::Resource res(anyExistingResourceUri);
res.properties();
}

The reason is following: In ResourceData::load(), m_cache is cleared and rewritten if m_cacheDirty is true. However the entry m_cache[NIE::url()] is not rewritten and therefore the following line in resetAll() fails:

 m_rm->m_urlKickOff.remove( m_cache[NIE::url()].toUrl() );

Changing that line to

m_rm->m_urlKickOff.remove( m_uri );

removes the problem and avoids the infinite loop.

(In reply to comment #7)
> (In reply to comment #3)
> > Created attachment 73051 [details]
> > Patch to remove calls to Resource::fromResourceUri() in
> > ResourceData::resetAll()
> 
> Unfortunately, I cannot commit this, cause it changes the signature of a
> method. That breaks compatibility with previous versions.

Just a thought: I guess in 5.0 you have to make the methods in ResourceWatcher URI based instead of taking Resource objects as arguments, because ResourceWatcher is in datamanagement now. It doesn't make sense that a class in datamanagement depends on Resource.

I think this has gotten fixed over time. Could you please cross check. I've tried to read through the entire log, but I'm a little confused as to what the issue is.

Thanks.

I'm marking this as NEEDSINFO - WAITINGFORINFO. The wait is for you to confirm if it has been fixed. If it has, then please mark it as fixed.

Cannot reproduce the crash in 4.9.3. Hard to say what fixed it...