[Nepomuk] Nepomuk Core - Questions & Patches

[Sebastian Trüg]

hm, maybe an idea: move the mutex locking to Resource. That could solve
the problem... what do you think?

On 05/28/2010 05:15 PM, Sebastian Trüg wrote:
> Hm, I think I found a situation where this could still fail:
> 
> Imagine this scenario:
> 
> the store contains resource n:/res/foo with identifier "foo"
> 
> We then use this code in one thread:
> 
> Resource r1(QUrl("n:/res/foo"));
> Resource r2("foo");
> 
> and this in another:
> 
> Resource r3("foo");
> 
> Now imagine for some reasons r1's determineUrl() has been called
> already, thus, calling it on r2 or r3 would result in a replace. So far
> so good.
> But what if both thread would try it at the same time? One of them would
> perform the determineUrl() and ultimately delete the ResourceData (which
> is referenced by both r2 and r3). At the same time r3's thread would
> wait for the mutex in determinUrl() to be released and then try to
> execute the method in the deleted ResourceData -> crash!
> 
> Cheers,
> Sebastian
> 
> On 05/27/2010 01:59 PM, Sebastian Trüg wrote:
>> Hi Vishesh,
>>
>> I finally looked at the remove-proxy patch again and fixed it. Now the
>> only problem left is the design weirdness of ResourceData instances
>> deleting themselves.
>>
>> The main thing I did was removing all calls to determineUri from
>> ResourceData and moving them into Resource. I did the same with load()
>> and store() which is actually not necessary. Maybe it would be cleaner
>> to move those back...
>>
>> I did not look into the merging of load and determineUri yet. I think we
>> should do that in a separate patch only after we cleaned this one up.
>> Otherwise it is impossible to debug.
>>
>> Cheers,
>> Sebastian
>>
>> On 05/26/2010 12:52 PM, Vishesh Handa wrote:
>>>
>>>
>>> On Wed, May 26, 2010 at 3:13 PM, Sebastian Trüg <trueg at kde.org
>>> <mailto:trueg at kde.org>> wrote:
>>>
>>>     On 05/26/2010 10:08 AM, Vishesh Handa wrote:
>>>     >     > 3. When checking if the the m_kickoffUri is a nie:url for a
>>>     resource.
>>>     >     > The nieUrl is assigned to be equal to the uri. This however
>>>     gets fixed
>>>     >     > in the load(). And nieUrl isn't used anywhere, so it doesn't
>>>     >     matter that
>>>     >     > much.
>>>     >
>>>     >     I suppose you mean this part:
>>>     >
>>>     >     if( it.next() ) {
>>>     >          QUrl uri = it["r"].uri();
>>>     >          if( uri.isEmpty() ) {
>>>     >             m_uri = m_kickoffUri;
>>>     >          }
>>>     >          else {
>>>     >             m_uri = uri;
>>>     >             m_nieUrl = uri;
>>>     >          }
>>>     >
>>>     >     The last two lines. AFAICT this is perfectly fine since in the
>>>     latter
>>>     >     case both nie:url and resource URI are equal.
>>>     >
>>>     >
>>>     > I respectfully disagree. :)
>>>     >
>>>     > The query used is this "select distinct ?r ?o where { { ?r nie:url
>>>     <uri>
>>>     > . } UNION { <uri> ?p ?o . }  } LIMIT 1". The case where ?r isn't empty
>>>     > is when the <uri> contains the nie:url and therefore ?r will
>>>     contain the
>>>     > resource uri.
>>>
>>>     You are of course correct. It should be "m_nieUrl = m_kickoffUri"
>>>     instead. Agreed?
>>>
>>>
>>> Yup. :)
>>>
>>> If you get the time could you please look at my horrible merge patch?
>>>
>>> Thanks
>>> - Vishesh Handa
>>>
>>>     Cheers,
>>>     Sebastian
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Nepomuk mailing list
>>> Nepomuk at kde.org
>>> https://mail.kde.org/mailman/listinfo/nepomuk
> _______________________________________________
> Nepomuk mailing list
> Nepomuk at kde.org
> https://mail.kde.org/mailman/listinfo/nepomuk
>