[Nepomuk] Nepomuk Core - Questions & Patches

[Sebastian Trüg]

Hehe, this does not help at all. Think about it: in my example there are
2 Resource instances involved. Thus: 2 mutexes which are locked
independent of each other. :)
The mutex is already there in ResourceData. It simply needs to be locked
in Resource instead of ResourceData::determineUri.

Cheers,
Sebastian

On 05/29/2010 01:19 PM, Vishesh Handa wrote:
> How about this?
> 
> I didn't know where to add the Mutex, and I didn't think ResourceData
> would be appropriate. So, I implemented the Resource::Private class and
> added it over there. This should not break binary compatibility as it
> already existed. Or is there a better way?
> 
> - Vishesh Handa
> 
> On Sat, May 29, 2010 at 2:25 PM, Vishesh Handa <handa.vish at gmail.com
> <mailto:handa.vish at gmail.com>> wrote:
> 
>     Hey Sebastian
> 
>     I'm sorry for the late reply I was busy with college stuff.
> 
>     I looked at the modified patch, and I like the fact that it works,
>     but I don't like the fact that we have to remember additional stuff.
>     DetermineUri() must not me called inside ResourceData, and that
>     you'll probably have to check if the ResourceData is stored() and
>     its uri has been determined, before calling a method. These small
>     things then to add up, and clutter the Resource Code.
> 
>     Ideally I would have liked ResourceData to do all this checking
>     itself, so that when altering the Resource class we don't have to
>     worry about it. ( But then we already had to worry about store() ) I
>     don't have a solution, but I'm trying to figure out one.
> 
>     I never even thought about multi-threading before this, and, yes, I
>     too think it will crash. ( Time to add more tests? Can we add
>     multi-threading tests? )  I was thinking of some way of checking if
>     the determineUri lock exists, and then accordingly calling
>     determineUri() from Resource, but I think your method of mutex
>     locking the Resource should cover it.
> 
>     I'll try to implement it (It pretty simple, right?)
> 
>     - Vishesh Handa
> 
> 
> 
> 
>     On Thu, May 27, 2010 at 5:29 PM, Sebastian Trüg <trueg at kde.org
>     <mailto:trueg at kde.org>> 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>
>         > <mailto: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
>         >
>         >
> 
> 
>