Hi Vishesh,
please find attached an updated version of the patch. There is still at
least one problem: run the resourcetest and see it crashing. This is due
to determineUri deleting "this". determineUri() is called all over the
place. I think the one solution is to move that call into Resource as I
already did with store().
The other thing is the comment in ResourceData::remove(). We need to
make sure that is still working.
Cheers,
Sebastian
On 05/18/2010 01:46 PM, Vishesh Handa wrote:
>
>
> On Tue, May 18, 2010 at 4:07 PM, Sebastian Trüg <[email protected]
> <mailto:[email protected]>> wrote:
>
> On 05/18/2010 12:30 PM, Vishesh Handa wrote:
> > 2. Don't we need to protect m_resources via a QMutex?
> >
> >
> > I'm currently blissfully unaware about concurrent programming. I
> assure
> > you I'm trying to learn, but till then could you take care of it?
>
> ok, will do.
>
> > [3. Wouldn't shoudBeDeleted() make more sense in
> > ResourceManagerPrivate?]
> >
> > :-/ It checks if a ResourceData should be deleted. I considered making
> > it static but this approach had less clutter. Could you please explain
> > why it should be in ResourceManagerPrivate?
>
> I thought that conceptually it is the resourcemanager maintaining the
> resourcedata objects. So it should also be the one to decide when one
> has to be deleted.
>
>
> Okay. I've changed it.
>
> The shouldBeDeleted function is currently called twice. Two times in
> Resource (destructor and operator==), and once in ResourceData::replaceWith.
>
> - Vishesh Handa
>
>
> > As for ResourceManagerPrivate: this is just how it is done in
> KDE: it is
> > a convention to name private classes that way. You can look in
> KDE and
> > Qt code all over the place and find these private classes.
> Their main
> > purpose is to keep as much member variables and methods out of
> the main
> > class as possible to make it possible to add and remove those
> without
> > breaking binary compatibility.
> >
> >
> > Yea. I know. The "pimpl" idiom. :)
> >
> > I will apply the patch and test it.
> >
> > Okay. I'll do the same.
> >
> > - Vishesh Handa
> >
> >
> > Cheers,
> > Sebastian
> >
> > On 05/16/2010 04:11 PM, Vishesh Handa wrote:
> > > Cleaned up the code a little bit.
> > > - fixed a small bug in Resource::~Resource(). (my fault)
> > > - m_resources is now managed along with reference counting.
> > > - Added a function ResourceData::shouldBeDeleted() to avoid
> > duplication
> > > of code.
> > >
> > > I think the shouldBeDeleted() function should be in another
> patch.
> > >
> > > Btw, this code still hasn't been extensively tested. I'll do the
> > testing
> > > later on. I just though I should share the patch so that I
> can get
> > your
> > > comments.
> > >
> > > - Vishesh Handa
> >
> >
>
>
Index: resourcemanager_p.h
===================================================================
--- resourcemanager_p.h (revision 1128573)
+++ resourcemanager_p.h (working copy)
@@ -97,7 +97,7 @@
*/
ResourceData* data( const QUrl& uri, const QUrl& type );
- bool dataCacheFull();
+ bool dataCacheFull() const;
/**
* Delete unused ResourceData objects from the cache.
@@ -115,6 +115,8 @@
*/
void determineAllUris();
+ bool shouldBeDeleted( ResourceData* rd ) const;
+
QList<ResourceData*> allResourceData();
QList<ResourceData*> allResourceDataOfType( const QUrl& type );
QList<ResourceData*> allResourceDataWithProperty( const QUrl& _uri,
const Variant& v );
Index: resourcedata.cpp
===================================================================
--- resourcedata.cpp (revision 1128573)
+++ resourcedata.cpp (working copy)
@@ -60,7 +60,6 @@
m_kickoffUri( uri ),
m_mainType( type ),
m_modificationMutex(QMutex::Recursive),
- m_proxyData(0),
m_cacheDirty(true),
m_pimoThing(0),
m_groundingOccurence(0),
@@ -102,16 +101,12 @@
QUrl Nepomuk::ResourceData::uri() const
{
- if( m_proxyData )
- return m_proxyData->uri();
return m_uri;
}
QUrl Nepomuk::ResourceData::type()
{
- if( m_proxyData )
- return m_proxyData->type();
load();
return m_mainType;
}
@@ -119,8 +114,6 @@
QList<QUrl> Nepomuk::ResourceData::allTypes()
{
- if( m_proxyData )
- return m_proxyData->allTypes();
load();
return m_types;
}
@@ -128,45 +121,30 @@
void Nepomuk::ResourceData::setTypes( const QList<QUrl>& types )
{
- if( m_proxyData ) {
- m_proxyData->setTypes( types );
- }
- else if ( store() ) {
- QMutexLocker lock(&m_modificationMutex);
+ QMutexLocker lock(&m_modificationMutex);
- // reset types
- m_types.clear();
- m_mainType = Soprano::Vocabulary::RDFS::Resource();
+ // reset types
+ m_types.clear();
+ m_mainType = Soprano::Vocabulary::RDFS::Resource();
- QList<Node> nodes;
- // load types (and set maintype)
- foreach( const QUrl& url, types ) {
- loadType( url );
- nodes << Node( url );
- }
-
- // update the data store
- MAINMODEL->updateProperty( m_uri, Soprano::Vocabulary::RDF::type(),
nodes );
+ QList<Node> nodes;
+ // load types (and set maintype)
+ foreach( const QUrl& url, types ) {
+ loadType( url );
+ nodes << Node( url );
}
+
+ // update the data store
+ MAINMODEL->updateProperty( m_uri, Soprano::Vocabulary::RDF::type(), nodes
);
}
void Nepomuk::ResourceData::resetAll( bool isDelete )
{
- // reset proxy
- bool hadProxy = false;
- if( m_proxyData ) {
- hadProxy = true;
- if( !m_proxyData->deref() &&
- rm()->dataCacheFull() )
- delete m_proxyData;
- m_proxyData = 0;
- }
-
// remove us from all caches (store() will re-insert us later if necessary)
m_rm->mutex.lock();
- if( !m_uri.isEmpty() && !hadProxy ) // if we had a proxy we were not in
m_initializedData ourselves
+ if( !m_uri.isEmpty() )
m_rm->m_initializedData.remove( m_uri );
if( !m_kickoffUri.isEmpty() )
m_rm->m_uriKickoffData.remove( m_kickoffUri );
@@ -197,20 +175,13 @@
QHash<QUrl, Nepomuk::Variant> Nepomuk::ResourceData::allProperties()
{
- if( m_proxyData )
- return m_proxyData->allProperties();
-
load();
-
return m_cache;
}
bool Nepomuk::ResourceData::hasProperty( const QUrl& uri )
{
- if( m_proxyData )
- return m_proxyData->hasProperty( uri );
-
if( !load() )
return false;
@@ -224,9 +195,6 @@
bool Nepomuk::ResourceData::hasProperty( const QUrl& p, const Variant& v )
{
- if( m_proxyData )
- return m_proxyData->hasProperty( p, v );
-
if( !load() )
return false;
@@ -246,9 +214,6 @@
bool Nepomuk::ResourceData::hasType( const QUrl& uri )
{
- if( m_proxyData )
- return m_proxyData->hasType( uri );
-
load();
return constHasType( uri );
}
@@ -274,9 +239,6 @@
Nepomuk::Variant Nepomuk::ResourceData::property( const QUrl& uri )
{
- if( m_proxyData )
- return m_proxyData->property( uri );
-
if ( load() ) {
// we need to protect the reading, too. load my be triggered from
another thread's
// connection to a Soprano statement signal
@@ -359,7 +321,7 @@
// that m_types contains more than one element: if we loaded them)
// The first type, however, can be set at creation time to any value
if ( m_mainType != Soprano::Vocabulary::RDFS::Resource() &&
- !MAINMODEL->containsAnyStatement( m_uri,
Soprano::Vocabulary::RDF::type(), m_mainType ) ) {
+ !MAINMODEL->containsAnyStatement( m_uri,
Soprano::Vocabulary::RDF::type(), m_mainType ) ) {
statements.append( Statement( m_uri, Soprano::Vocabulary::RDF::type(),
m_mainType ) );
}
@@ -476,54 +438,48 @@
{
Q_ASSERT( uri.isValid() );
- if( m_proxyData )
- return m_proxyData->setProperty( uri, value );
+ QMutexLocker lock(&m_modificationMutex);
- // step 0: make sure this resource is in the store
- if ( store() ) {
- QMutexLocker lock(&m_modificationMutex);
+ QList<Node> valueNodes;
- QList<Node> valueNodes;
-
- // make sure resource values are in the store
- if ( value.simpleType() == qMetaTypeId<Resource>() ) {
- QList<Resource> l = value.toResourceList();
- for( QList<Resource>::iterator resIt = l.begin(); resIt !=
l.end(); ++resIt ) {
- resIt->m_data->store();
- }
+ // make sure resource values are in the store
+ if ( value.simpleType() == qMetaTypeId<Resource>() ) {
+ QList<Resource> l = value.toResourceList();
+ for( QList<Resource>::iterator resIt = l.begin(); resIt != l.end();
++resIt ) {
+ resIt->m_data->store();
}
+ }
- // add the actual property statements
+ // add the actual property statements
- // one-to-one Resource
- if( value.isResource() ) {
- valueNodes.append( value.toUrl() );
- }
+ // one-to-one Resource
+ if( value.isResource() ) {
+ valueNodes.append( value.toUrl() );
+ }
- // one-to-many Resource
- else if( value.isResourceList() ) {
- const QList<QUrl>& l = value.toUrlList();
- for( QList<QUrl>::const_iterator resIt = l.constBegin(); resIt !=
l.constEnd(); ++resIt ) {
- valueNodes.append( *resIt );
- }
+ // one-to-many Resource
+ else if( value.isResourceList() ) {
+ const QList<QUrl>& l = value.toUrlList();
+ for( QList<QUrl>::const_iterator resIt = l.constBegin(); resIt !=
l.constEnd(); ++resIt ) {
+ valueNodes.append( *resIt );
}
+ }
- // one-to-many literals
- else if( value.isList() ) {
- valueNodes = Nepomuk::valuesToRDFNodes( value );
- }
+ // one-to-many literals
+ else if( value.isList() ) {
+ valueNodes = Nepomuk::valuesToRDFNodes( value );
+ }
- // one-to-one literal
- else {
- valueNodes.append( Nepomuk::valueToRDFNode( value ) );
- }
+ // one-to-one literal
+ else {
+ valueNodes.append( Nepomuk::valueToRDFNode( value ) );
+ }
- // update the cache for now
- m_cache[uri] = value;
+ // update the cache for now
+ m_cache[uri] = value;
- // update the store
- MAINMODEL->updateProperty( m_uri, uri, valueNodes );
- }
+ // update the store
+ MAINMODEL->updateProperty( m_uri, uri, valueNodes );
}
@@ -531,9 +487,6 @@
{
Q_ASSERT( uri.isValid() );
- if( m_proxyData )
- return m_proxyData->removeProperty( uri );
-
QMutexLocker lock(&m_modificationMutex);
if ( determineUri() ) {
@@ -557,18 +510,12 @@
// That way the proxy handling will take care of the rest.
m_rm->determineAllUris();
- if( m_proxyData ) {
- m_proxyData->remove( recursive );
- }
- else {
- QMutexLocker lock(&m_modificationMutex);
+ QMutexLocker lock(&m_modificationMutex);
-
- if ( determineUri() ) {
- MAINMODEL->removeAllStatements( Statement( m_uri, Node(), Node() )
);
- if ( recursive ) {
- MAINMODEL->removeAllStatements( Statement( Node(), Node(),
m_uri ) );
- }
+ if ( determineUri() ) {
+ MAINMODEL->removeAllStatements( Statement( m_uri, Node(), Node() ) );
+ if ( recursive ) {
+ MAINMODEL->removeAllStatements( Statement( Node(), Node(), m_uri )
);
}
}
@@ -578,9 +525,6 @@
bool Nepomuk::ResourceData::exists()
{
- if( m_proxyData )
- return m_proxyData->exists();
-
if( determineUri() ) {
return MAINMODEL->containsAnyStatement( Statement( m_uri, Node(),
Node() ) );
}
@@ -591,166 +535,170 @@
bool Nepomuk::ResourceData::isValid() const
{
- if( m_proxyData )
- return m_proxyData->isValid();
-
return( !m_mainType.isEmpty() && ( !m_uri.isEmpty() ||
!m_kickoffUri.isEmpty() || !m_kickoffId.isEmpty() ) );
}
bool Nepomuk::ResourceData::determineUri()
{
- if( m_proxyData )
- return m_proxyData->determineUri();
+ //
+ // Preconditions:
+ // 1. m_kickoffId is not a local file path or URL (use m_kickoffUri for
that)
+ //
+ // Now for the hard part
+ // We have the following possible situations:
+ // 1. m_uri is already valid
+ // -> simple, nothing to do
+ //
+ // 2. m_uri is not valid
+ // -> we need to determine the URI
+ //
+ // 2.1. m_kickoffId is valid
+ // 2.1.1. m_kickoffId is a resource URI
+ // -> use it as m_uri
+ // 2.1.2. m_kickoffId is a nao:identifier for r
+ // -> use r as m_uri
+ // 2.1.3. m_kickoffId is not found
+ // -> create new random m_uri and save m_kickoffId as nao:identifier
+ //
+ // 2.2. m_kickoffUri is valid
+ // 2.2.1. it is a file URL
+ // 2.2.1.1. it is nie:url for r
+ // -> use r as m_uri
+ // 2.2.1.2. it points to a file on a removable device for which we have a
filex:/ URL
+ // -> use the r in r nie:url filex:/...
+ // 2.2.1.3. it is a file which is not an object in some nie:url relation
+ // -> create new random m_uri and use kickoffUriOrId() as m_nieUrl
+ //
+ QMutexLocker lock(&m_determineUriMutex);
- else {
- //
- // Preconditions:
- // 1. m_kickoffId is not a local file path or URL (use m_kickoffUri
for that)
- //
- // Now for the hard part
- // We have the following possible situations:
- // 1. m_uri is already valid
- // -> simple, nothing to do
- //
- // 2. m_uri is not valid
- // -> we need to determine the URI
- //
- // 2.1. m_kickoffId is valid
- // 2.1.1. m_kickoffId is a resource URI
- // -> use it as m_uri
- // 2.1.2. m_kickoffId is a nao:identifier for r
- // -> use r as m_uri
- // 2.1.3. m_kickoffId is not found
- // -> create new random m_uri and save m_kickoffId as
nao:identifier
- //
- // 2.2. m_kickoffUri is valid
- // 2.2.1. it is a file URL
- // 2.2.1.1. it is nie:url for r
- // -> use r as m_uri
- // 2.2.1.2. it points to a file on a removable device for which we
have a filex:/ URL
- // -> use the r in r nie:url filex:/...
- // 2.2.1.3. it is a file which is not an object in some nie:url
relation
- // -> create new random m_uri and use kickoffUriOrId() as
m_nieUrl
- //
- QMutexLocker lock(&m_determineUriMutex);
+ if( m_uri.isEmpty() ) {
- if( m_uri.isEmpty() ) {
+ Soprano::Model* model = MAINMODEL;
- Soprano::Model* model = MAINMODEL;
-
- if( !m_kickoffId.isEmpty() ) {
- //
- // The kickoffUriOrId is actually a URI
- //
- KUrl uriFromKickoffId(m_kickoffId);
- if( model->containsAnyStatement( uriFromKickoffId, Node(),
Node() )) {
- m_uri = uriFromKickoffId;
- }
-
- //
- // Check if the kickoffUriOrId is a resource identifier or a
resource URI
- //
- else {
- QString query = QString::fromLatin1("select distinct ?r ?o
where { { ?r %1 %2 . } UNION { %3 ?p ?o . } . } LIMIT 1")
- .arg(
Soprano::Node::resourceToN3(Soprano::Vocabulary::NAO::identifier()) )
- .arg(
Soprano::Node::literalToN3(m_kickoffId) )
- .arg(
Soprano::Node::resourceToN3(KUrl(m_kickoffId)) );
- Soprano::QueryResultIterator it = model->executeQuery(
query, Soprano::Query::QueryLanguageSparql );
- if( it.next() ) {
- m_uri = it["r"].uri();
- if( m_uri.isEmpty() ) {
- m_uri = KUrl(m_kickoffId);
- }
- it.close();
- }
- }
+ if( !m_kickoffId.isEmpty() ) {
+ //
+ // The kickoffUriOrId is actually a URI
+ //
+ KUrl uriFromKickoffId(m_kickoffId);
+ if( model->containsAnyStatement( uriFromKickoffId, Node(), Node()
)) {
+ m_uri = uriFromKickoffId;
}
- else if( !m_kickoffUri.isEmpty() ) {
- //
- // In one query determine if the URI is already used as
resource URI or as
- // nie:url
- //
- QString query = QString::fromLatin1("select distinct ?r ?o
where { "
- "{ ?r %1 %2 . } "
- "UNION "
- "{ %2 ?p ?o . } "
- "} LIMIT 1")
- .arg(
Soprano::Node::resourceToN3(Nepomuk::Vocabulary::NIE::url()) )
- .arg(
Soprano::Node::resourceToN3(m_kickoffUri) );
+ //
+ // Check if the kickoffUriOrId is a resource identifier or a
resource URI
+ //
+ else {
+ QString query = QString::fromLatin1("select distinct ?r ?o
where { { ?r %1 %2 . } UNION { %3 ?p ?o . } . } LIMIT 1")
+ .arg(
Soprano::Node::resourceToN3(Soprano::Vocabulary::NAO::identifier()) )
+ .arg( Soprano::Node::literalToN3(m_kickoffId) )
+ .arg(
Soprano::Node::resourceToN3(KUrl(m_kickoffId)) );
Soprano::QueryResultIterator it = model->executeQuery( query,
Soprano::Query::QueryLanguageSparql );
if( it.next() ) {
- QUrl uri = it["r"].uri();
- if( uri.isEmpty() ) {
- m_uri = m_kickoffUri;
+ m_uri = it["r"].uri();
+ if( m_uri.isEmpty() ) {
+ m_uri = KUrl(m_kickoffId);
}
- else {
- m_uri = uri;
- m_nieUrl = uri;
- }
it.close();
}
- else if( m_kickoffUri.scheme() == QLatin1String("file") ) {
- //
- // This is the hard part: The file may still be saved
using a filex:/ URL
- // We have to determine if that is the case. For that we
need to look if the URL
- // starts with any of the mounted filesystems' mount paths
and then reconstruct
- // the filex:/ URL. Since that is quite some work which
involved Solid and, thus,
- // DBus calls we simply call the removable storage service
directly.
- //
- // If there is no resource yet we create a new uri in
store()
- //
+ }
+ }
- //
- // We create a new dbus connection to protect us from
multi-thread related crashes.
- //
- KUrl resourceUri = QDBusReply<QString>(
QDBusInterface(QLatin1String("org.kde.nepomuk.services.nepomukremovablestorageservice"),
-
QLatin1String("/nepomukremovablestorageservice"),
-
QLatin1String("org.kde.nepomuk.RemovableStorage"),
-
DBusConnectionPool::threadConnection())
- .call(
QLatin1String("resourceUriFromLocalFileUrl"),
-
m_kickoffUri.url() ) ).value();
- if( !resourceUri.isEmpty() ) {
- m_uri = resourceUri;
- }
-
- m_nieUrl = m_kickoffUri;
- }
- else if( m_kickoffUri.scheme() == QLatin1String("nepomuk") ) {
- // for nepomuk URIs we simply use the kickoff URI as
resource URI
+ else if( !m_kickoffUri.isEmpty() ) {
+ //
+ // In one query determine if the URI is already used as resource
URI or as
+ // nie:url
+ //
+ QString query = QString::fromLatin1("select distinct ?r ?o where {
"
+ "{ ?r %1 %2 . } "
+ "UNION "
+ "{ %2 ?p ?o . } "
+ "} LIMIT 1")
+ .arg(
Soprano::Node::resourceToN3(Nepomuk::Vocabulary::NIE::url()) )
+ .arg( Soprano::Node::resourceToN3(m_kickoffUri) );
+ Soprano::QueryResultIterator it = model->executeQuery( query,
Soprano::Query::QueryLanguageSparql );
+ if( it.next() ) {
+ QUrl uri = it["r"].uri();
+ if( uri.isEmpty() ) {
m_uri = m_kickoffUri;
}
else {
- // for everything else we use m_kickoffUri as nie:url with
a new random m_uri
- m_nieUrl = m_kickoffUri;
+ m_uri = uri;
+ m_nieUrl = uri;
}
+ it.close();
}
+ else if( m_kickoffUri.scheme() == QLatin1String("file") ) {
+ //
+ // This is the hard part: The file may still be saved using a
filex:/ URL
+ // We have to determine if that is the case. For that we need
to look if the URL
+ // starts with any of the mounted filesystems' mount paths and
then reconstruct
+ // the filex:/ URL. Since that is quite some work which
involved Solid and, thus,
+ // DBus calls we simply call the removable storage service
directly.
+ //
+ // If there is no resource yet we create a new uri in store()
+ //
+ //
+ // We create a new dbus connection to protect us from
multi-thread related crashes.
+ //
+ KUrl resourceUri = QDBusReply<QString>(
QDBusInterface(QLatin1String("org.kde.nepomuk.services.nepomukremovablestorageservice"),
+
QLatin1String("/nepomukremovablestorageservice"),
+
QLatin1String("org.kde.nepomuk.RemovableStorage"),
+
DBusConnectionPool::threadConnection())
+ .call(
QLatin1String("resourceUriFromLocalFileUrl"),
+
m_kickoffUri.url() ) ).value();
+ if( !resourceUri.isEmpty() ) {
+ m_uri = resourceUri;
+ }
+
+ m_nieUrl = m_kickoffUri;
+ }
+ else if( m_kickoffUri.scheme() == QLatin1String("nepomuk") ) {
+ // for nepomuk URIs we simply use the kickoff URI as resource
URI
+ m_uri = m_kickoffUri;
+ }
else {
- // no kickoff values. We will only create a new random URI in
store()
- return false;
+ // for everything else we use m_kickoffUri as nie:url with a
new random m_uri
+ m_nieUrl = m_kickoffUri;
}
+ }
- //
- // Move us to the final data hash now that the URI is known
- //
- if( !m_uri.isEmpty() ) {
- QMutexLocker rmlock(&m_rm->mutex);
+ else {
+ // no kickoff values. We will only create a new random URI in
store()
+ return false;
+ }
- ResourceDataHash::iterator it =
m_rm->m_initializedData.find(m_uri);
- if( it == m_rm->m_initializedData.end() ) {
- m_rm->m_initializedData.insert( m_uri, this );
- }
- else {
- m_proxyData = it.value();
- m_proxyData->ref();
- }
+ //
+ // Move us to the final data hash now that the URI is known
+ //
+ if( !m_uri.isEmpty() ) {
+ QMutexLocker rmlock(&m_rm->mutex);
+
+ ResourceDataHash::iterator it =
m_rm->m_initializedData.find(m_uri);
+ if( it == m_rm->m_initializedData.end() ) {
+ m_rm->m_initializedData.insert( m_uri, this );
}
+ else {
+ // As the ResourceData already exists in m_initializedData, we
simply
+ // make all the the Resources which contain "this"
ResourceData, point
+ // to the initialized ResourceData instead.
+ replaceWith( it.value() );
+ }
}
+ }
- return !m_uri.isEmpty();
+ return !m_uri.isEmpty();
+}
+
+
+void Nepomuk::ResourceData::replaceWith( Nepomuk::ResourceData* rd )
+{
+ Q_FOREACH( Resource* res, m_resources ) {
+ res->m_data = rd;
+ rd->ref( res );
}
+ delete this;
}
@@ -795,17 +743,13 @@
bool Nepomuk::ResourceData::operator==( const ResourceData& other ) const
{
- const ResourceData* that = this;
- if( m_proxyData )
- that = m_proxyData;
-
- if( that == &other )
+ if( this == &other )
return true;
- return( that->m_uri == other.m_uri &&
- that->m_mainType == other.m_mainType &&
- that->m_kickoffUri == other.m_kickoffUri &&
- that->m_kickoffId == other.m_kickoffId );
+ return( m_uri == other.m_uri &&
+ m_mainType == other.m_mainType &&
+ m_kickoffUri == other.m_kickoffUri &&
+ m_kickoffId == other.m_kickoffId );
}
@@ -817,8 +761,7 @@
m_uri.url(),
m_mainType.toString())
.arg(m_ref);
- if(m_proxyData)
- dbg << QLatin1String(", proxy: " ) << *m_proxyData;
+
return dbg << QLatin1String("]");
}
Index: resourcemanager.cpp
===================================================================
--- resourcemanager.cpp (revision 1128573)
+++ resourcemanager.cpp (working copy)
@@ -84,12 +84,9 @@
}
if( it == end ) {
- ResourceData* d = new ResourceData( url, QString(), type, this );
-// kDebug() << "--------------------------- Created new ResourceData:"
<< *d;
- return d;
+ return new ResourceData( url, QString(), type, this );
}
else {
-// kDebug() << "---------------------------- Reusing" << *it.value()
<< "for" << uri;
return it.value();
}
}
@@ -111,8 +108,7 @@
KickoffDataHash::iterator it = m_idKickoffData.find( uriOrId );
if( it == m_idKickoffData.end() ) {
- ResourceData* d = new ResourceData( QUrl(), uriOrId, type, this );
- return d;
+ return new ResourceData( QUrl(), uriOrId, type, this );
}
else {
return it.value();
@@ -197,7 +193,7 @@
}
-bool Nepomuk::ResourceManagerPrivate::dataCacheFull()
+bool Nepomuk::ResourceManagerPrivate::dataCacheFull() const
{
return dataCnt >= 1000;
}
@@ -230,6 +226,20 @@
}
+bool Nepomuk::ResourceManagerPrivate::shouldBeDeleted( ResourceData* rd ) const
+{
+ if ( rd->cnt() )
+ return false;
+
+ //
+ // We delete data objects in one of two cases:
+ // 1. They are not valid and as such not in one of the
ResourceManagerPrivate kickoff lists
+ // 2. The cache is already full and we need to clean up
+ //
+ return( !rd->isValid() || dataCacheFull() );
+}
+
+
void Nepomuk::ResourceManagerPrivate::_k_storageServiceInitialized( bool
success )
{
if( success ) {
Index: resourcedata.h
===================================================================
--- resourcedata.h (revision 1128573)
+++ resourcedata.h (working copy)
@@ -25,6 +25,7 @@
#include <QtCore/QList>
#include <QtCore/QHash>
#include <QtCore/QMutex>
+#include <QtCore/QMutexLocker>
#include <QtCore/QAtomicInt>
#include "variant.h"
@@ -37,6 +38,7 @@
namespace Nepomuk {
+ class Resource;
class ResourceManagerPrivate;
class ResourceData
@@ -45,11 +47,16 @@
explicit ResourceData( const QUrl& uri, const QString& kickoffId_,
const QUrl& type_, ResourceManagerPrivate* rm );
~ResourceData();
- inline bool ref() {
+ inline bool ref(Nepomuk::Resource* res) {
+ QMutexLocker lock(&m_resourcesMutex);
+ m_resources.push_back( res );
return m_ref.ref();
}
- inline bool deref() {
+
+ inline bool deref(Nepomuk::Resource* res) {
+ QMutexLocker lock(&m_resourcesMutex);
+ m_resources.removeAll( res );
return m_ref.deref();
}
@@ -113,6 +120,8 @@
* This is also the only place where a new URI is generated via
ResourceManager::generateUniqueUri()
* in case m_uri is empty.
*
+ * Be aware that store() may replace this ResourceData entirely and
delete it!
+ *
* \sa exists, setProperty
*/
bool store();
@@ -155,7 +164,7 @@
ResourceManagerPrivate* rm() const { return m_rm; }
- ResourceData* proxy() const { return m_proxyData; }
+ bool shouldBeDeleted() const;
private:
void loadType( const QUrl& type );
@@ -187,13 +196,15 @@
QMutex m_determineUriMutex;
mutable QMutex m_modificationMutex;
+ /// Contains a list of resources which contain this ResourceData
+ QList<Resource*> m_resources;
+ QMutex m_resourcesMutex;
+
/**
- * Used to virtually merge two data objects representing the same
- * resource. This happens if the resource was once created using its
- * actual URI and once via its ID. To prevent early loading we allow
- * this scenario.
+ * Makes the m_data pointer of all the Resources in m_resources point
to
+ * rd, instead of 'this'. Handles refernce counting as well.
*/
- ResourceData* m_proxyData;
+ void replaceWith( ResourceData* rd );
QHash<QUrl, Variant> m_cache;
bool m_cacheDirty;
Index: resource.cpp
===================================================================
--- resource.cpp (revision 1128573)
+++ resource.cpp (working copy)
@@ -47,14 +47,16 @@
Nepomuk::Resource::Resource()
{
m_data = ResourceManager::instance()->d->data( QUrl(), QUrl() );
- m_data->ref();
+ if ( m_data )
+ m_data->ref( this );
}
Nepomuk::Resource::Resource( ResourceManager* manager )
{
m_data = manager->d->data( QUrl(), QUrl() );
- m_data->ref();
+ if ( m_data )
+ m_data->ref( this );
}
@@ -62,7 +64,7 @@
{
m_data = res.m_data;
if ( m_data )
- m_data->ref();
+ m_data->ref( this );
}
@@ -70,7 +72,7 @@
{
m_data = ResourceManager::instance()->d->data( uri, type );
if ( m_data )
- m_data->ref();
+ m_data->ref( this );
}
@@ -78,7 +80,7 @@
{
m_data = manager->d->data( uri, type );
if ( m_data )
- m_data->ref();
+ m_data->ref( this );
}
@@ -86,7 +88,7 @@
{
m_data = ResourceManager::instance()->d->data( uri, type );
if ( m_data )
- m_data->ref();
+ m_data->ref( this );
}
@@ -94,7 +96,7 @@
{
m_data = ResourceManager::instance()->d->data( uri, type );
if ( m_data )
- m_data->ref();
+ m_data->ref( this );
}
@@ -102,7 +104,7 @@
{
m_data = manager->d->data( uri, type );
if ( m_data )
- m_data->ref();
+ m_data->ref( this );
}
@@ -110,26 +112,16 @@
{
m_data = data;
if ( m_data )
- data->ref();
+ m_data->ref( this );
}
Nepomuk::Resource::~Resource()
{
if ( m_data ) {
- if ( !m_data->deref() ) {
- //
- // We delete data objects in one of three cases:
- // 1. They are not valid and as such not in one of the
ResourceManagerPrivate kickoff lists
- // 2. They have a proxy which is the actual thing to reuse later on
- // 3. The cache is already full and we need to clean up
- //
- if ( !m_data->isValid() ||
- m_data->proxy() ||
- m_data->rm()->dataCacheFull() ) {
- delete m_data;
- }
- }
+ m_data->deref( this );
+ if ( m_data->rm()->shouldBeDeleted( m_data ) )
+ delete m_data;
}
}
@@ -137,12 +129,12 @@
Nepomuk::Resource& Nepomuk::Resource::operator=( const Resource& res )
{
if( m_data != res.m_data ) {
- if ( m_data && !m_data->deref() && !m_data->isValid() ) {
+ if ( m_data && !m_data->deref( this ) &&
m_data->rm()->shouldBeDeleted( m_data ) ) {
delete m_data;
}
m_data = res.m_data;
if ( m_data )
- m_data->ref();
+ m_data->ref( this );
}
return *this;
@@ -209,8 +201,9 @@
void Nepomuk::Resource::setTypes( const QList<QUrl>& types )
{
- if ( m_data )
+ if ( m_data && m_data->store() ) {
m_data->setTypes( types );
+ }
}
@@ -311,7 +304,7 @@
void Nepomuk::Resource::setProperty( const QUrl& uri, const Nepomuk::Variant&
value )
{
- if ( m_data ) {
+ if ( m_data && m_data->store() ) {
m_data->setProperty( uri, value );
}
}
_______________________________________________
Nepomuk mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/nepomuk
