Skip to content

Make DNS operations non-blocking when using c-ares #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make DNS operations non-blocking when using c-ares #261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 25 additions & 25 deletions src/conn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ static void _conn_sm_handle_stanza(xmpp_conn_t *const conn,
static unsigned short _conn_default_port(xmpp_conn_t *conn,
xmpp_conn_type_t type);
static void _conn_reset(xmpp_conn_t *conn);
static int _conn_connect(xmpp_conn_t *conn,
const char *domain,
xmpp_conn_type_t type,
xmpp_conn_handler callback,
void *userdata);
static int _conn_preconnect(xmpp_conn_t *conn,
char *domain,
xmpp_conn_type_t type,
xmpp_conn_handler callback,
void *userdata);
static void _send_valist(xmpp_conn_t *conn,
const char *fmt,
va_list ap,
Expand Down Expand Up @@ -709,14 +709,14 @@ int xmpp_connect_client(xmpp_conn_t *conn,

if (conn->xsock)
sock_free(conn->xsock);
conn->xsock = sock_new(conn, domain, altdomain, altport);
if (!conn->xsock)
goto err_mem;

rc = _conn_connect(conn, domain, XMPP_CLIENT, callback, userdata);
strophe_free(conn->ctx, domain);
/* domain ownership transfers to conn */
if ((rc =
_conn_preconnect(conn, domain, XMPP_CLIENT, callback, userdata))) {
return rc;
}

return rc;
return sock_new(conn, domain, altdomain, altport);

err_mem:
strophe_free(conn->ctx, domain);
Expand Down Expand Up @@ -752,6 +752,7 @@ int xmpp_connect_component(xmpp_conn_t *conn,
xmpp_conn_handler callback,
void *userdata)
{
int rc;
/* The server domain, jid and password MUST be specified. */
if (!(server && conn->jid && conn->pass))
return XMPP_EINVOP;
Expand All @@ -769,13 +770,15 @@ int xmpp_connect_component(xmpp_conn_t *conn,
port = port ? port : _conn_default_port(conn, XMPP_COMPONENT);
if (conn->xsock)
sock_free(conn->xsock);
conn->xsock = sock_new(conn, NULL, server, port);
if (!conn->xsock)
return XMPP_EMEM;

/* JID serves as an identifier here and will be used as "to" attribute
of the stream */
return _conn_connect(conn, conn->jid, XMPP_COMPONENT, callback, userdata);
if ((rc = _conn_preconnect(conn, strophe_strdup(conn->ctx, conn->jid),
XMPP_COMPONENT, callback, userdata))) {
return rc;
}

return sock_new(conn, NULL, server, port);
}

/** Initiate a raw connection to the XMPP server.
Expand Down Expand Up @@ -2170,11 +2173,11 @@ static void _conn_reset(xmpp_conn_t *conn)
handler_system_delete_all(conn);
}

static int _conn_connect(xmpp_conn_t *conn,
const char *domain,
xmpp_conn_type_t type,
xmpp_conn_handler callback,
void *userdata)
static int _conn_preconnect(xmpp_conn_t *conn,
char *domain,
xmpp_conn_type_t type,
xmpp_conn_handler callback,
void *userdata)
{
xmpp_open_handler open_handler;

Expand All @@ -2186,14 +2189,10 @@ static int _conn_connect(xmpp_conn_t *conn,
_conn_reset(conn);

conn->type = type;
conn->domain = strophe_strdup(conn->ctx, domain);
conn->domain = domain;
if (!conn->domain)
return XMPP_EMEM;

conn->sock = sock_connect(conn->xsock);
Copy link
Contributor Author

@singpolyma singpolyma Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this happens as part of sock_new now

if (conn->sock == INVALID_SOCKET)
return XMPP_EINT;

/* setup handler */
conn->conn_handler = callback;
conn->userdata = userdata;
Expand All @@ -2208,6 +2207,7 @@ static int _conn_connect(xmpp_conn_t *conn,
* hard to fix, since we'd have to detect and fire off the callback
* from within the event loop */

conn->sock = INVALID_SOCKET;
conn->state = XMPP_STATE_CONNECTING;
conn->timeout_stamp = time_stamp();

Expand Down
46 changes: 25 additions & 21 deletions src/event.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,22 +57,19 @@
#include "strophe.h"
#include "common.h"
#include "parser.h"
#include "resolver.h"

#ifndef STROPHE_MESSAGE_BUFFER_SIZE
/** Max buffer size for receiving messages. */
#define STROPHE_MESSAGE_BUFFER_SIZE 4096
#endif

static int _connect_next(xmpp_conn_t *conn)
static void _connect_next(xmpp_conn_t *conn)
{
sock_close(conn->sock);
conn->sock = sock_connect(conn->xsock);
if (conn->sock == INVALID_SOCKET)
return -1;

conn->sock = INVALID_SOCKET;
conn->timeout_stamp = time_stamp();

return 0;
sock_connect(conn->xsock);
}

/** Run the event loop once.
Expand Down Expand Up @@ -220,17 +217,14 @@ void xmpp_run_once(xmpp_ctx_t *ctx, unsigned long timeout)

/* make sure the timeout hasn't expired */
if (time_elapsed(conn->timeout_stamp, time_stamp()) <=
conn->connect_timeout)
FD_SET(conn->sock, &wfds);
else {
conn->connect_timeout) {
if (conn->sock != INVALID_SOCKET)
FD_SET(conn->sock, &wfds);
} else {
strophe_info(ctx, "xmpp", "Connection attempt timed out.");
ret = _connect_next(conn);
if (ret != 0) {
conn->error = ETIMEDOUT;
Copy link
Contributor Author

@singpolyma singpolyma Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as written, this error is no longer produced and will always end up as XMPP_EINT instead

conn_disconnect(conn);
} else {
_connect_next(conn);
if (conn->sock != INVALID_SOCKET)
FD_SET(conn->sock, &wfds);
}
}
break;
case XMPP_STATE_CONNECTED:
Expand All @@ -254,6 +248,15 @@ void xmpp_run_once(xmpp_ctx_t *ctx, unsigned long timeout)
connitem = connitem->next;
}

#ifdef HAVE_CARES
if (ares_chan) {
int ares_max = ares_fds(ares_chan, &rfds, &wfds);
Copy link
Member

@sjaeckel sjaeckel Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@singpolyma singpolyma Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (ares_max > max)
max = ares_max;
ares_timeout(ares_chan, &tv, &tv);
}
#endif

/* check for events */
if (max > 0)
ret = select(max + 1, &rfds, &wfds, NULL, &tv);
Expand All @@ -271,6 +274,11 @@ void xmpp_run_once(xmpp_ctx_t *ctx, unsigned long timeout)
return;
}

#ifdef HAVE_CARES
if (ares_chan)
ares_process(ares_chan, &rfds, &wfds);
Copy link
Member

@sjaeckel sjaeckel Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

#endif

/* no events happened */
if (ret == 0 && tls_read_bytes == 0)
return;
Expand All @@ -292,11 +300,7 @@ void xmpp_run_once(xmpp_ctx_t *ctx, unsigned long timeout)
/* connection failed */
strophe_debug(ctx, "xmpp", "connection failed, error %d",
ret);
ret = _connect_next(conn);
if (ret != 0) {
conn->error = ret;
conn_disconnect(conn);
}
_connect_next(conn);
break;
}

Expand Down
Loading