[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [Libevent-users] evhttp client error handling

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. 

* 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.