Make DNS operations non-blocking when using c-ares #261
Conversation
singpolyma
force-pushed
the
async-connect
branch
from
f0ef433 to
5485155
Compare
| strophe_info(ctx, "xmpp", "Connection attempt timed out."); | ||
| ret = _connect_next(conn); | ||
| if (ret != 0) { | ||
| conn->error = ETIMEDOUT; |
There was a problem hiding this comment.
as written, this error is no longer produced and will always end up as XMPP_EINT instead
| if (!conn->domain) | ||
| return XMPP_EMEM; | ||
|
|
||
| conn->sock = sock_connect(conn->xsock); |
There was a problem hiding this comment.
this happens as part of sock_new now
| ares_library_init(ARES_LIB_INIT_ALL); | ||
| rc = ares_init(&ares_chan); | ||
| if (rc != ARES_SUCCESS) { | ||
| /* This is bad */ |
There was a problem hiding this comment.
This is called from xmpp_initialize which we could change to support returning an error?
There was a problem hiding this comment.
My ares.h says the following:
CARES_EXTERN CARES_DEPRECATED_FOR(ares_init_options) int ares_init(
ares_channel_t **channelptr);
Shouldn't we then directly go with the new API?
This is called from xmpp_initialize which we could change to support returning an error?
I guess we should do something, yes!
... also ares_library_init() potentially returns an error ... and there's ares_library_init_mem() which could somehow fit with the custom memory allocators already supported in this library, but won't quite fit in reality ... which makes me think whether we could somehow leave this custom init up to the user of libstrophe?
There was a problem hiding this comment.
And have the user call xmpp_initialize with an ares_chan? It feels like pushing a lot onto the user when they almost certainly want the setup we would give them. But we could do it that way if you want, I was mostly following the existing ares code and just trying to make it non blocking.
singpolyma
force-pushed
the
async-connect
branch
4 times, most recently
from
f0d0bb1 to
a30e9da
Compare
singpolyma
force-pushed
the
async-connect
branch
2 times, most recently
from
44bea41 to
164c6ed
Compare
Previously the connection and reconnection process would block the event loop waiting on DNS results. Without c-ares this is still the case here, but when c-ares is used this is now a non-blocking operation which progresses during normal xmpp_run_once calls along with everything else in the system.
singpolyma
force-pushed
the
async-connect
branch
from
164c6ed to
1b01f28
Compare
| ares_library_init(ARES_LIB_INIT_ALL); | ||
| rc = ares_init(&ares_chan); | ||
| if (rc != ARES_SUCCESS) { | ||
| /* This is bad */ |
There was a problem hiding this comment.
My ares.h says the following:
CARES_EXTERN CARES_DEPRECATED_FOR(ares_init_options) int ares_init(
ares_channel_t **channelptr);
Shouldn't we then directly go with the new API?
This is called from xmpp_initialize which we could change to support returning an error?
I guess we should do something, yes!
... also ares_library_init() potentially returns an error ... and there's ares_library_init_mem() which could somehow fit with the custom memory allocators already supported in this library, but won't quite fit in reality ... which makes me think whether we could somehow leave this custom init up to the user of libstrophe?
|
|
||
| #ifdef HAVE_CARES | ||
| if (ares_chan) { | ||
| int ares_max = ares_fds(ares_chan, &rfds, &wfds); |
There was a problem hiding this comment.
hmm, ares.h says the following:
CARES_EXTERN CARES_DEPRECATED_FOR(
ARES_OPT_EVENT_THREAD or
ARES_OPT_SOCK_STATE_CB) int ares_fds(const ares_channel_t *channel,
fd_set *read_fds, fd_set *write_fds);
shouldn't one of these new constructs be used then?
Just to be clear: I'm only looking at the c-ares header&source files and don't have any experience with it (yet).
There was a problem hiding this comment.
ares_fds is deprecated because they suggest to not use select and instead use a background thread or epoll, etc (which the other options enable). This is good advice in general because of the limitations of select, however libstrophe currently has a hard reliance on select in the event loop anyway so I think the advice doesn't really apply to us here.
|
|
||
| #ifdef HAVE_CARES | ||
| if (ares_chan) | ||
| ares_process(ares_chan, &rfds, &wfds); |
There was a problem hiding this comment.
Same here
CARES_EXTERN CARES_DEPRECATED_FOR(ares_process_fds) void ares_process(
ares_channel_t *channel, fd_set *read_fds, fd_set *write_fds);
2 participants
Previously the connection and reconnection process would block the event loop waiting on DNS results. Without c-ares this is still the case here, but when c-ares is used this is now a non-blocking operation which progresses during normal xmpp_run_once calls along with everything else in the system.
Fixes #260