Re: Regarding Async DNS resolver
Date: Mon, 22 Jul 2019 18:08:31 +0530
Hi Daniel,
I have done the changes as per your suggestion - that is, to create socket
pair during async thread initialization, return the read socket fd to
client application and write dummy data to signal client that socket is
readable.
I did the testing on version which I am using (7.57.11) and verified the
changes.
Could you please review the attached patch file (generated after merging
the changes to latest libcurl) ? .
Regards,
Amit Katyal
On Wed, Jul 17, 2019 at 5:14 AM Amit <itsamitkatyal_at_gmail.com> wrote:
>
>
> On Wed, Jul 17, 2019 at 4:13 AM Daniel Stenberg <daniel_at_haxx.se> wrote:
>
>> On Tue, 16 Jul 2019, Amit wrote:
>>
>> >> I don't actually advocate using 100ms all the time. It needs to be
>> shorter
>> >> at first and then increase over time. curl_multi_timeout() return such
>> >> timeouts.
>> >
>> > Thanks, will use curl_multi_timeout() instead of fixed 100 msec
>> > timer.
>>
>> Just note that curl_multi_timeout() only started to return those suitable
>> timeouts during the threaded name resolver in 7.60.0, so in earlier
>> versions
>> you need to do that yourself.
>>
>> OK, will check if it is straightforward to back port the required
> changes.
>
>
>> > why do you think notifying DNS-completed in resolver thread is a bad
>> idea ?
>>
>> Because it would be surprising to applications (since no callbacks in
>> libcurl
>> is ever called from another thread) and introduce the typical threading
>> issues
>> requiring mutexes, locks etc and risk causing race conditions in the app.
>>
>>
>
>
>> > Anyways, application would be handling that DNS-completed event in CURL
>> > thread context.
>>
>> If you call libcurl from thread A and the callback comes from thread B
>> (that
>> curl itself created), then surely those are not using the same thread
>> contexts?
>>
>> Yes, Callback will be called from another thread context (created by
> cur) but client will simply post an event
> to curl thread in the callback so that synchronization is not
> required. On receiving the event, curl thread will
> perform necessary action in its own context. This will ensure that
> curl API's are only called in single thread context.
> Do you see any issues with above approach ? or your concern is that
> you don't want to define/restrict the callback
> implementation behavior and would prefer the curl implementation to
> be as per its existing design (which doesn't
> recommend calling callback in another thread context as I have seen
> callback being called in the curl thread context ) ?
>
>
>> >> The *better* fix would be to instead use a pipe/sockpair to signal
>> that the
>> >> name resolve is complete and then have the application be able to wait
>> on
>> >> that.
>>
>> > Are you suggesting that let CURL internally create pipe/socket pair for
>> DNS
>> > resolution and application wait on the same fd ?
>>
>> I'm suggesting that would be one way to offer a solution.
>>
>> > And on DNS resolution, CURL will write to the pipe, which will cause
>> > application thread to wake up and application can trigger the CURL to
>> resume
>> > the connection ?
>>
>> Correct.
>>
>> > Regarding your concern on Windows, I am not sure about the CURL
>> development
>> > process, but since I am working on linux, would like to know if It if
>> okay
>> > to implement it only for linux ? If you are fine, then I can explore
>> more in
>> > this direction.
>>
>> Sure, a solution that only works for non-Windows is totally fine, as
>> Windows
>> users could then either just remain the current method or a Windows
>> developer
>> could work on providing the corresponding solution for that platform.
>>
>> As I am new to curl, It would be great if you can provide me some
> pointer for similar kind of implementation already
> available in the curl, so that I can refer the same for this
> use-case.
>
>> --
>>
>> / daniel.haxx.se | Get the best commercial curl support there is -
>> from me
>> | Private help, bug fixes, support, ports, new features
>> | https://www.wolfssl.com/contact/
>>
>
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
- application/octet-stream attachment: dns.patch