Re: [Libevent-users] evhttp client error handling

[Azat Khuzhin <dohardgopro@xxxxxxxxx>]

On Sat, Jan 19, 2013 at 9:41 AM, Patrick Pelletier
<ppelletier@xxxxxxxxxx> wrote:
> On 11/19/2012 06:48 AM, Nick Mathewson wrote:
>
>> On Thu, Nov 15, 2012 at 2:32 AM, Patrick Pelletier
>> <ppelletier@xxxxxxxxxx <mailto:ppelletier@xxxxxxxxxx>> wrote:
>> [...]
>>
>>     Not sure what the proper solution is for this, though.  I'm not
>>     enough of a sockets expert.  Although I'm curious why we need to
>>     create the socket before we've looked up the name.  If we looked up
>>     the name first, then we could create the socket with the proper
>>     address family in the first place.
>>
>>
>> My first thought would be that binding before doing the name lookup
>> isn't going to work: we need to find out what address family we're going
>> to connect to before we can find out what address family we're going to
>> bind to?
>
>
> I definitely agree that is what ought to be happening.  Unfortunately, I'm
> having trouble understanding the code well enough in order to submit a patch
> to make it do that.
>
> Here's what I've figured out so far:
>
> * In http.c, evhttp_connection_connect_ calls bind_socket, which creates the
> socket (with AF_INET, which is what we don't want).  The file descriptor is
> stored in evcon->fd, and also stored in the bufferevent with
> bufferevent_setfd.

Hi, if I understand correct, "AF_INET" is not installed always.

First "make_addrinfo()" is called in "bind_socket()" and installing "AF_UNSPEC"
https://github.com/libevent/libevent/blob/0c2bacca43ffe452d4f5cdc4b57fd29a7795777d/http.c#L4057

With means that and IPV4 and IPV6 is allowed.

Than only if "ai" is NULL in "bind_socket_ai()", "AF_INET" is installed. in
https://github.com/libevent/libevent/blob/0c2bacca43ffe452d4f5cdc4b57fd29a7795777d/http.c#L4018

But "ai" can't be NULL in "bind_socket_ai()", see
https://github.com/libevent/libevent/blob/0c2bacca43ffe452d4f5cdc4b57fd29a7795777d/http.c#L4088

>
> * Then, evhttp_connection_connect_ calls bufferevent_socket_connect_hostname
>
> * In bufferevent_sock.c, bufferevent_socket_connect_hostname calls
> evutil_getaddrinfo_async_, with bufferevent_connect_getaddrinfo_cb as the
> callback
>
> * bufferevent_connect_getaddrinfo_cb calls bufferevent_socket_connect
>
> * bufferevent_socket_connect checks if fd < 0, and calls evutil_socket to
> create the socket if necessary
>
> So, at first glance, it seems like we could avoid creating the socket in
> evhttp_connection_connect_, and just wait for bufferevent_socket_connect to
> create the socket for us, after it has done the name lookup, and knows what
> the proper address family is.
>
> The only potential problem seemed to be that the http code wants the file
> descriptor in evcon->fd, in addition to being in the buffer event.
>
> So, I tried the following:
>
> * Commented out the lines from "bind_socket" through "bufferevent_setfd" in
> evhttp_connection_connect_
>
> * In evhttp_error_cb (which seems to be a callback for more than just
> errors), I added "evcon->fd = bufferevent_getfd(bufev);" in the (currently
> empty) if branch for what == BEV_EVENT_CONNECTED, under the theory that this
> would compensate for not having set evcon->fd in evhttp_connection_connect_
>
> However, when I did this, the tests failed horribly, so there's obviously
> something I don't understand about what's happening and the order in which
> it needs to happen.
>
>
>> Or alternatively, if we have already bound to a socket, we need to limit
>> our name lookup to the address family we bound to.  If we've bound to
>> 127.0.0.1, there's no point in doing an AF_UNSPEC name lookup -- we can
>> only connect to AF_INET addresses in that case.
>
>
> That's a potential workaround that would allow localhost (or any dual-stack
> host) to work, but it would still mean that evhttp client connections are
> always IPv4-only, and wouldn't work with IPv6-only hosts.  So it's better
> than the current situation, but not entirely satisfying.
>
>
> --Patrick
>
> ***********************************************************************
> To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
> unsubscribe libevent-users    in the body.



--
Azat Khuzhin
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.