Review Request: Do not terminate threads

[Dawit A]

On Sat, Aug 6, 2011 at 7:46 AM, Albert Astals Cid <aacid at kde.org> wrote:
> On Dijous 04 Agost 2011 15:28:49 Dawit A wrote:
>> On Thu, Aug 4, 2011 at 5:31 AM, Albert Astals Cid <tsdgeos at terra.es> wrote:
>> >    This is an automatically generated e-mail. To reply, visit:
>> > http://git.reviewboard.kde.org/r/102179/
>> >
>> > On August 4th, 2011, 3:19 a.m., *Dawit Alemayehu* wrote:
>> >
>> > 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 expensive 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
>> > lookup thread gets terminated is if your nameserver takes longer than
>> > 1000 ms per query. That is because the two KUriFilterPlugins that use it
>> > set a timeout value of that duration. Otherwise, that code path should
>> > not be encountered at all!
>> >
>> >  So you do not want a patch that fixes konqueror crashing 75% times i use
>> >  it in exchange of having a small memory leak once in a blue moon?
>> >  Awesome.
>> >
>> Did I say I do not want the patch ??
>
> That's what i understood when reading your comment, sorry if it is not what
> you meant.

Well, it is always misunderstandings that cause issues. I am sure I
too would have misunderstood my response if I had bothered to re-read
it. :) Sorry about that. It did not come out how I intended it to come
out.

>> All I said was need to think of a
>> better way to fix this patch! Clam down.
>
> Since you do not want a mutex and you do not want my current solution either I
> can not think of much more ways of fixing this.
>
> Maybe by doing what Thiago suggests and instead of starting a short lived
> thread each time start a long lived one, though we will still need the mutex
> if we want it to be 100% correct.

I think the event loop based solution is probably the way to go. It is
what both the localdomain and fixhost uri filter plugins used to use
before I refactored the code out and put in the parent class to reduce
the DNS lookups and also avoid code duplication. Anyhow, I have posted
a patch that uses the aforementioned event loop solution at
https://git.reviewboard.kde.org/r/102238/. Can you please see if it
resolves your issues ?

> If you are at the Berlin Desktop Summit maybe we can meet and try to find a
> solution together. You can usually find me surrounded by a bunch of other
> spaniards.

Unfortunately, I won't be there.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kio_hostinfo.patch
Type: application/octet-stream
Size: 6485 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110807/2650192c/attachment.obj>