Review Request: Do not terminate threads

[Albert Astals Cid]

> On Aug. 11, 2011, 9:41 a.m., David Faure wrote:
> > kio/kio/hostinfo.cpp, line 236
> > <http://git.reviewboard.kde.org/r/102179/diff/5/?file=31680#file31680line236>
> >
> >     Maybe move nameLookupThread as a member of hostInfoAgentPrivate, to avoid multiplying the global statics (and therefore have more control over order of destruction)? Or does this make no sense?

Could make that, but they seem somewhat separate things to me so i'd prefer to keep them separate unless you have a strong feeling about it


> On Aug. 11, 2011, 9:41 a.m., David Faure wrote:
> > kio/kio/hostinfo.cpp, line 224
> > <http://git.reviewboard.kde.org/r/102179/diff/5/?file=31680#file31680line224>
> >
> >     You could also just create the worker on the stack here.

Done


> On Aug. 11, 2011, 9:41 a.m., David Faure wrote:
> > kio/kio/hostinfo.cpp, line 179
> > <http://git.reviewboard.kde.org/r/102179/diff/5/?file=31680#file31680line179>
> >
> >     reviewboard is highlighting quite a number of lines with trailing whitespace

Fixed


> On Aug. 11, 2011, 9:41 a.m., David Faure wrote:
> > kio/kio/hostinfo.cpp, line 266
> > <http://git.reviewboard.kde.org/r/102179/diff/5/?file=31680#file31680line266>
> >
> >     Ouch, a busy loop.
> >     
> >     The standard solution is to use ... yes ... a semaphore :-) I love semaphores.
> >     
> >     Acquire the semaphore here, release it in the thread once the worker object has been created.

Changed.


> On Aug. 11, 2011, 9:41 a.m., David Faure wrote:
> > kio/kio/hostinfo.cpp, line 177
> > <http://git.reviewboard.kde.org/r/102179/diff/5/?file=31680#file31680line177>
> >
> >     The interesting question is, what if hostname was already in the m_lookups map?
> >     
> >     Either we want to detect this upfront and not create a second request, or we need a "list of requests for a given hostname" in the map.

Found out i can actually use the request id as key so we should not have this problem now.


- Albert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102179/#review5623
-----------------------------------------------------------


On Aug. 11, 2011, 10:32 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. 11, 2011, 10:32 a.m.)
> 
> 
> Review request for kdelibs and Dawit Alemayehu.
> 
> 
> Summary
> -------
> 
> Each time the terminate code triggers my Konqueror crashes, i'm substituting the terminate for just waiting the thread to finish and 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.
> 
> 
> Diffs
> -----
> 
>   kio/kio/hostinfo.cpp 344b1d8 
> 
> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110811/842186c2/attachment.htm>