[Nepomuk] Nepomuk Core - Questions & Patches

[Vishesh Handa]

Hey Sebastian

On Sun, May 23, 2010 at 7:06 PM, Sebastian Trüg <trueg at kde.org> wrote:

> 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.
>
>
Well then we need a better way of solving the "delete this" problem. When
you say moving "it" into Resource, do you mean determineUri() or
replaceWith() ?

- Vishesh Handa


> > *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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/nepomuk/attachments/20100523/4f1cf3aa/attachment.htm