[Nepomuk] Nepomuk Core - Questions & Patches

[Sebastian Trüg]

Hi Vishesh,

On 05/23/2010 02:28 PM, Vishesh Handa wrote:
> 
> Hey Sebastian
> 
>     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().
> 
> 
> I had no idea there were tests. For some reason I totally missed them! I
> was going to write my own tests to validate the patch.
> 
> Anyway, you're right. It crashes. The problem is that replaceWith(..)
> deletes itself. And then determineUri tries to access the m_uri
> variable. Even if we simply "return true", from determineUri(). It is
> called all over the place and hence some other member function accesses
> its, no longer valid, member variables. (Your explanation wasn't clear
> enough!)
> 
> There is one more problem. I can't always reproduce it. Before
> replaceWith(..) is called the ResourceManager's mutex is locked. Then
> when deleting it self resetSelf(bool) attempts to lock the mutex. The
> locking sometimes fails, and it just keeps waiting.
> 
> One way of solving both the problems is to derive ResourceData from
> QObject. And then call deleteLater() instead of "delete this". (Patch
> attached)

oh, no, please don't. Totally useless overhead. It is much simpler to
make the mutex recursive.

> *Random Question:* In determineUri()'s last if condition -
> 
> 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() );
>             }
>         }
> 
> Couldn't we move the MutexLocker inside the if( it ==
> m_rm->m_initializedData.end() ) ?

no, that is not possible since m_initializedData could change while the
mutex is not locked. Thus, we need to make sure we iterate while locking.

Cheers,
Sebastian