<div dir="ltr"><div><div><div><div>From: <b class="gmail_sendername">Michael Pyne</b> <span dir="ltr"><<a href="mailto:mpyne@kde.org">mpyne@kde.org</a>></span><br>Date: Thu, Oct 29, 2015 at 7:23 AM<br>Subject: Re: Debug ThreadWeaver::IdDecorator<br>To: <a href="mailto:kde-devel@kde.org">kde-devel@kde.org</a><br><br><br><span class="">On Wed, October 28, 2015 18:45:40 Olivier Churlaud wrote:<br>
> Le 28/10/2015 16:08, Thomas Lübking a écrit :<br>
> > On Mittwoch, 28. Oktober 2015 14:33:02 CEST, Olivier Churlaud wrote:<br>
> >> Hello<br>
> >><br>
> >> I finally managed to install the Threadweaver in debug mod.<br>
> >><br>
> >> Fulltrace in gdb : <a href="https://paste.kde.org/pdnashan2" rel="noreferrer" target="_blank">https://paste.kde.org/pdnashan2</a><br>
> >><br>
> >>                  in valgrind <a href="https://paste.kde.org/pwooi9alf" rel="noreferrer" target="_blank">https://paste.kde.org/pwooi9alf</a><br>
> >><br>
> >> Thx for your hints<br>
> ><br>
> > W/o even looking at the code (is it pushed somewhere at all?) - you're<br>
> > not explicitly deleting and autodelete (by default) IdDecorator, are you?<br>
> ><br>
> > Cheers,<br>
> > Thomas<br>
><br>
> The code is pushed on amarok, branch kf5.<br>
><br>
> IdDecorator is never mentioned anywhere in the code. Only derived classes.<br>
> It works well in the KDE4 environnement (since the amarok devs are able<br>
> to compile it and running it well) but not on the KF5 one. That's why I<br>
> asked it there, because it might be a modification in the library that<br>
> I'm not aware of.<br>
<br>
</span>It seems to me that there is a double-deletion going on, likely due to a<br>
manual deletion of the ThreadWeaver job after it is completed at src / core-<br>
impl / collections / db / sql / SqlQueryMaker.cpp:234, and likely also at line<br>
160 with similar reasoning.<br>
<br>
Line 234 contains<br>
<br>
> void<br>
> SqlQueryMaker::done( ThreadWeaver::JobPointer job )<br>
> {<br>
><br>
>     ThreadWeaver::QObjectDecorator *qs = new<br>
>     ThreadWeaver::QObjectDecorator(job.data()); qs->deleteLater();<br>
>     d->worker = 0; // d->worker *is* the job, prevent stale pointer<br>
>     emit queryDone();<br>
><br>
> }<br>
<br>
In this case ThreadWeaver::JobPointer is a QSharedPointer which means that the<br>
QSharedPointer controls the object lifetime for this sql worker thread.<br>
However the pointer is manually pulled out of the QSharedPointer (by<br>
job.data()) and then manually deleted.<br>
<br>
By the time the actual QSharedPointer is ready to delete the pointer, it has<br>
already been deleted thanks to the qs->deleteLater(), which causes the crash<br>
you see.<br>
<br>
This is also where you see the "IdDecorator" stuff from: IdDecorator is a<br>
superclass of the QObjectDecorator that was wrapped around d->worker.<br>
<br>
>From the moment that d->worker was enqueue()'d into ThreadWeaver (in line 212)<br>
Amarok lost control of its object lifetime, and therefore should not attempt<br>
to delete it manually.<br>
<br>
Instead just let ThreadWeaver control the object lifetime. If you wish for the<br>
code to work in both the ThreadWeaver and non-ThreadWeaver cases you should at<br>
least also check for d->blocking when manually deleting d->worker.<br>
<br>
Regards,<br>
 - Michael Pyne<br><br><br></div>As we couldn't  figure out how this threadweaver issue could be solved, olivier wrote a mail to kde-devel regarding the same <br></div>. Michael pyne pointed out wherer the issue was .Please refer to the above message ,he has explained the reason of why we were getting the segmentation fault .<br><br><br></div>Thank you ,<br></div>rishabh <br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 30, 2015 at 12:27 AM, Aroonav Mishra <span dir="ltr"><<a href="mailto:aroonav11@gmail.com" target="_blank">aroonav11@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div>Hello all,<br><br></div>KF5::ThreadWeaver is not very well documented and the docs available for it's porting is inadequate. And due to this, porting this part was tricky & took up quite a bit of my time.<br><br></div>I will be explaining all the changes that I made in <a href="https://quickgit.kde.org/?p=amarok.git&a=blobdiff&h=15c1b0b28d8865a56b37da04e919e6e803a3454d&hp=b4427f2db1126f05bb10b57216a47070f14acfe3&f=src%2Fcore-impl%2Fcollections%2Fdb%2Fsql%2FSqlQueryMaker.cpp&hb=adee016bdeb5ea747e45f88beb42c58f4521d492" target="_blank">here</a>. (which also includes some of the changes that I have mentioned <a href="https://mail.kde.org/pipermail/amarok-devel/2015-July/013486.html" target="_blank">here</a>)<br><br></div>In the constructor call,ThreadWeaver::Job no longer inherits QObject in KF5. Hence it only seemed logical to add QObject into the inheritance hierarchy(and subsequently the constructor call too).<br><br>The pure virtual function <span>ThreadWeaver</span>::Job::run() prototype has changed
 and hence 2 new formal parameters have been added in the SqlWorkerThread. I had set a default value to each of these 2 new parameters. I neither saw a benefit nor a way of using these new parameters. The first default parameter is QSharedPointer<TheradWeaver::Job>() which returns a QSharedPointer pointing to null. The second default parameter is self-explanatory. The Q_UNUSED has been added to suppress the warnings of non-usage of these parameters.<br><br></div>defaultBegin(), defaultEnd() and the signals(started, done, failed) have been removed in KF5::ThreadWeaver::Job and these have been moved into QObjectDecorator class. QObjectDecorator is a wrapper class that wraps around the Job object. I faced problems in using QObjectDecorator & hence I had directly added these(the functions and the signals) into the SqlWorkerThread class itself.<br><br>d->worker->deleteLater(); This is one part which I hadn't paid much attention before until now. Rishabh has identified the issue in the recent review <a href="https://git.reviewboard.kde.org/r/125872/" target="_blank">request #125872</a> to be here. I have suggested the modifications in the comments.<br><br></div><div>Thank you,<br></div><div>Aroonav Mishra.<br></div></div>
<br>_______________________________________________<br>
Amarok-devel mailing list<br>
<a href="mailto:Amarok-devel@kde.org">Amarok-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/amarok-devel" rel="noreferrer" target="_blank">https://mail.kde.org/mailman/listinfo/amarok-devel</a><br>
<br></blockquote></div><br></div>