From kde-core-devel Thu Aug 04 03:19:40 2011 From: "Dawit Alemayehu" Date: Thu, 04 Aug 2011 03:19:40 +0000 To: kde-core-devel Subject: Re: Review Request: Do not terminate threads Message-Id: <20110804031940.25378.26310 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kde-core-devel&m=131242801601743 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============0802111358512411039==" --===============0802111358512411039== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5369 ----------------------------------------------------------- I do not like this patch for the very reason you stated. I do not want the = mutex there either because it is rather expensive. As it stands we start a = thread for each lookup right now which in of itself is already too expensiv= e for my taste. Hence, I will have to think of some other way to avoid the = even worse solution of creating a local even loop. = Having said that, are you using a slow DNS server ? The only way the looku= p thread gets terminated is if your nameserver takes longer than 1000 ms pe= r query. That is because the two KUriFilterPlugins that use it set a timeou= t value of that duration. Otherwise, that code path should not be encounter= ed at all! - Dawit On Aug. 2, 2011, 11:30 a.m., Albert Astals Cid wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102179/ > ----------------------------------------------------------- > = > (Updated Aug. 2, 2011, 11:30 a.m.) > = > = > Review request for kdelibs and Dawit Alemayehu. > = > = > Summary > ------- > = > Each time the terminate code triggers my Konqueror crashes, i'm substitut= ing the terminate for just waiting the thread to finish and we just ignorin= g it. > = > The code has a race condition in which wait() returns false, then we swit= ch to the thread and m_autoDelete is still not set and thus noone will dele= te the thread. I can add a mutex if you guys think this is unacceptable. > = > = > Diffs > ----- > = > kio/kio/hostinfo.cpp fef39fc = > = > Diff: http://git.reviewboard.kde.org/r/102179/diff > = > = > Testing > ------- > = > When the = > kDebug() << "Name look up for" << hostName << "failed"; > if triggers i do not get a crash anymore. > = > = > Thanks, > = > Albert > = > --===============0802111358512411039== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/102179/ 

I do not l=
ike this patch for the very reason you stated. I do not want the mutex ther=
e either because it is rather expensive. As it stands we start a thread for=
 each lookup right now which in of itself is already too expensive for my t=
aste. Hence, I will have to think of some other way to avoid the even worse=
 solution of creating a local even loop. =


Having said that,  are you using a slow DNS server ? The only way the looku=
p thread gets terminated is if your nameserver takes longer than 1000 ms pe=
r query. That is because the two KUriFilterPlugins that use it set a timeou=
t value of that duration. Otherwise, that code path should not be encounter=
ed at all!

- Dawit


On August 2nd, 2011, 11:30 a.m., Albert Astals Cid wrote:

Review request for kdelibs and Dawit Alemayehu.
By Albert Astals Cid.

Updated Aug. 2, 2011, 11:30 a.m.

Descripti= on 

Each time the terminate code triggers my Konqueror crashes, =
i'm substituting the terminate for just waiting the thread to finish an=
d we just ignoring it.

The code has a race condition in which wait() returns false, then we switch=
 to the thread and m_autoDelete is still not set and thus noone will delete=
 the thread. I can add a mutex if you guys think this is unacceptable.

Testing <= /h1> 
When the =

kDebug() << "Name look up for" << hostName << &=
quot;failed";
if triggers i do not get a crash anymore.

Diffs=

  • kio/kio/hostinfo.cpp (fef39fc)

View Diff
--===============0802111358512411039==--