<br><br><div class="gmail_quote">On Sat, May 29, 2010 at 5:46 PM, Sebastian Trüg <span dir="ltr"><<a href="mailto:trueg@kde.org">trueg@kde.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Hehe, this does not help at all. Think about it: in my example there are<br>
2 Resource instances involved. Thus: 2 mutexes which are locked<br>
independent of each other. :)<br>
The mutex is already there in ResourceData. It simply needs to be locked<br>
in Resource instead of ResourceData::determineUri.<br>
<br></blockquote><div><br>Uhh I'm confused. Why don't you handle the multi-threading? <br><br>I should really learn about multi-threading. If you have a couple of spare minutes could you explain why my method won't work?<br>
<br>My rationale -<br><br>Thread 1 :<br><div style="margin-left: 40px;">Resource r1("foo");<br>r1.property( nao:numericRating )<br>-> the mutex is locked<br>-> performs whatever and determines the uri<br><br>
</div>Thread 2 :<br><div style="margin-left: 40px;">Resource r2("foo");<br>r2.setProperty( whatever )<br>-> the mutex can't get locked so it waits till it does<br>-> mutex now locked. Thread 1 should have determined the uri by now<br>
-> perform operation<br></div><br>Thanks<br>- Vishesh Handa<br><br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Cheers,<br>
<font color="#888888">Sebastian<br>
</font><div class="im"><br>
On 05/29/2010 01:19 PM, Vishesh Handa wrote:<br>
> How about this?<br>
><br>
> I didn't know where to add the Mutex, and I didn't think ResourceData<br>
> would be appropriate. So, I implemented the Resource::Private class and<br>
> added it over there. This should not break binary compatibility as it<br>
> already existed. Or is there a better way?<br>
><br>
> - Vishesh Handa<br>
><br>
> On Sat, May 29, 2010 at 2:25 PM, Vishesh Handa <<a href="mailto:handa.vish@gmail.com">handa.vish@gmail.com</a><br>
</div><div class="im">> <mailto:<a href="mailto:handa.vish@gmail.com">handa.vish@gmail.com</a>>> wrote:<br>
><br>
> Hey Sebastian<br>
><br>
> I'm sorry for the late reply I was busy with college stuff.<br>
><br>
> I looked at the modified patch, and I like the fact that it works,<br>
> but I don't like the fact that we have to remember additional stuff.<br>
> DetermineUri() must not me called inside ResourceData, and that<br>
> you'll probably have to check if the ResourceData is stored() and<br>
> its uri has been determined, before calling a method. These small<br>
> things then to add up, and clutter the Resource Code.<br>
><br>
> Ideally I would have liked ResourceData to do all this checking<br>
> itself, so that when altering the Resource class we don't have to<br>
> worry about it. ( But then we already had to worry about store() ) I<br>
> don't have a solution, but I'm trying to figure out one.<br>
><br>
> I never even thought about multi-threading before this, and, yes, I<br>
> too think it will crash. ( Time to add more tests? Can we add<br>
> multi-threading tests? ) I was thinking of some way of checking if<br>
> the determineUri lock exists, and then accordingly calling<br>
> determineUri() from Resource, but I think your method of mutex<br>
> locking the Resource should cover it.<br>
><br>
> I'll try to implement it (It pretty simple, right?)<br>
><br>
> - Vishesh Handa<br>
><br>
><br>
><br>
><br>
> On Thu, May 27, 2010 at 5:29 PM, Sebastian Trüg <<a href="mailto:trueg@kde.org">trueg@kde.org</a><br>
</div><div class="im">> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>> wrote:<br>
><br>
> Hi Vishesh,<br>
><br>
> I finally looked at the remove-proxy patch again and fixed it.<br>
> Now the<br>
> only problem left is the design weirdness of ResourceData instances<br>
> deleting themselves.<br>
><br>
> The main thing I did was removing all calls to determineUri from<br>
> ResourceData and moving them into Resource. I did the same with<br>
> load()<br>
> and store() which is actually not necessary. Maybe it would be<br>
> cleaner<br>
> to move those back...<br>
><br>
> I did not look into the merging of load and determineUri yet. I<br>
> think we<br>
> should do that in a separate patch only after we cleaned this<br>
> one up.<br>
> Otherwise it is impossible to debug.<br>
><br>
> Cheers,<br>
> Sebastian<br>
><br>
> On 05/26/2010 12:52 PM, Vishesh Handa wrote:<br>
> ><br>
> ><br>
> > On Wed, May 26, 2010 at 3:13 PM, Sebastian Trüg <<a href="mailto:trueg@kde.org">trueg@kde.org</a><br>
> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>><br>
</div><div><div></div><div class="h5">> > <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>>> wrote:<br>
> ><br>
> > On 05/26/2010 10:08 AM, Vishesh Handa wrote:<br>
> > > > 3. When checking if the the m_kickoffUri is a<br>
> nie:url for a<br>
> > resource.<br>
> > > > The nieUrl is assigned to be equal to the uri.<br>
> This however<br>
> > gets fixed<br>
> > > > in the load(). And nieUrl isn't used anywhere, so<br>
> it doesn't<br>
> > > matter that<br>
> > > > much.<br>
> > ><br>
> > > I suppose you mean this part:<br>
> > ><br>
> > > if( it.next() ) {<br>
> > > QUrl uri = it["r"].uri();<br>
> > > if( uri.isEmpty() ) {<br>
> > > m_uri = m_kickoffUri;<br>
> > > }<br>
> > > else {<br>
> > > m_uri = uri;<br>
> > > m_nieUrl = uri;<br>
> > > }<br>
> > ><br>
> > > The last two lines. AFAICT this is perfectly fine<br>
> since in the<br>
> > latter<br>
> > > case both nie:url and resource URI are equal.<br>
> > ><br>
> > ><br>
> > > I respectfully disagree. :)<br>
> > ><br>
> > > The query used is this "select distinct ?r ?o where { {<br>
> ?r nie:url<br>
> > <uri><br>
> > > . } UNION { <uri> ?p ?o . } } LIMIT 1". The case where<br>
> ?r isn't empty<br>
> > > is when the <uri> contains the nie:url and therefore ?r will<br>
> > contain the<br>
> > > resource uri.<br>
> ><br>
> > You are of course correct. It should be "m_nieUrl =<br>
> m_kickoffUri"<br>
> > instead. Agreed?<br>
> ><br>
> ><br>
> > Yup. :)<br>
> ><br>
> > If you get the time could you please look at my horrible merge<br>
> patch?<br>
> ><br>
> > Thanks<br>
> > - Vishesh Handa<br>
> ><br>
> > Cheers,<br>
> > Sebastian<br>
> ><br>
> ><br>
><br>
><br>
><br>
</div></div></blockquote></div><br>