ares: remove fd from multi fd set when ares is about to close the fd

When using c-ares for asyn dns, the dns socket fd was silently closed
by c-ares without curl being aware. curl would then 'realize' the fd
has been removed at next call of Curl_resolver_getsock, and only then
notify the CURLMOPT_SOCKETFUNCTION to remove fd from its poll set with
CURL_POLL_REMOVE. At this point the fd is already closed.

By using ares socket state callback (ARES_OPT_SOCK_STATE_CB), this
patch allows curl to be notified that the fd is not longer needed
for neither for write nor read. At this point by calling
Curl_multi_closed we are able to notify multi with CURL_POLL_REMOVE
before the fd is actually closed by ares.

In asyn-ares.c Curl_resolver_duphandle we can't use ares_dup anymore
since it does not allow passing a different sock_state_cb_data

Closes #3238

diff --git a/lib/asyn-ares.c b/lib/asyn-ares.c
index 5cfb2602df71b82ff8ef60edd989f0b06c464240..6a49566c869385c887dd6b45b98da5a435312528 100644 (file)
--- a/lib/asyn-ares.c
+++ b/lib/asyn-ares.c
@@ -119,6 +119,17 @@ void Curl_resolver_global_cleanup(void)
 #endif
 }
 
+
+static void Curl_ares_sock_state_cb(void *data, ares_socket_t socket_fd,
+                                    int readable, int writable)
+{
+  struct Curl_easy *easy = data;
+  if(!readable && !writable) {
+    DEBUGASSERT(easy);
+    Curl_multi_closed(easy, socket_fd);
+  }
+}
+
 /*
  * Curl_resolver_init()
  *
@@ -126,9 +137,14 @@ void Curl_resolver_global_cleanup(void)
  * URL-state specific environment ('resolver' member of the UrlState
  * structure).  Fills the passed pointer by the initialized ares_channel.
  */
-CURLcode Curl_resolver_init(void **resolver)
+CURLcode Curl_resolver_init(struct Curl_easy *easy, void **resolver)
 {
-  int status = ares_init((ares_channel*)resolver);
+  int status;
+  struct ares_options options;
+  int optmask = ARES_OPT_SOCK_STATE_CB;
+  options.sock_state_cb = Curl_ares_sock_state_cb;
+  options.sock_state_cb_data = easy;
+  status = ares_init_options((ares_channel*)resolver, &options, optmask);
   if(status != ARES_SUCCESS) {
     if(status == ARES_ENOMEM)
       return CURLE_OUT_OF_MEMORY;
@@ -159,12 +175,15 @@ void Curl_resolver_cleanup(void *resolver)
  * environment ('resolver' member of the UrlState structure).  Duplicates the
  * 'from' ares channel and passes the resulting channel to the 'to' pointer.
  */
-int Curl_resolver_duphandle(void **to, void *from)
+CURLcode Curl_resolver_duphandle(struct Curl_easy *easy, void **to, void *from)
 {
-  /* Clone the ares channel for the new handle */
-  if(ARES_SUCCESS != ares_dup((ares_channel*)to, (ares_channel)from))
-    return CURLE_FAILED_INIT;
-  return CURLE_OK;
+  (void)from;
+  /*
+   * it would be better to call ares_dup instead, but right now
+   * it is not possible to set 'sock_state_cb_data' outside of
+   * ares_init_options
+   */
+  return Curl_resolver_init(easy, to);
 }
 
 static void destroy_async_data(struct Curl_async *async);

diff --git a/lib/asyn-thread.c b/lib/asyn-thread.c
index 2a59294e5c877aac2ad5aa9ef58fd71ce76dfd22..4c8f88b149b2668bad654ad0f7643269dc2bf941 100644 (file)
--- a/lib/asyn-thread.c
+++ b/lib/asyn-thread.c
@@ -108,8 +108,9 @@ void Curl_resolver_global_cleanup(void)
  * URL-state specific environment ('resolver' member of the UrlState
  * structure).
  */
-CURLcode Curl_resolver_init(void **resolver)
+CURLcode Curl_resolver_init(struct Curl_easy *easy, void **resolver)
 {
+  (void)easy;
   *resolver = calloc(1, sizeof(struct resdata));
   if(!*resolver)
     return CURLE_OUT_OF_MEMORY;
@@ -132,10 +133,10 @@ void Curl_resolver_cleanup(void *resolver)
  * Called from curl_easy_duphandle() to duplicate resolver URL state-specific
  * environment ('resolver' member of the UrlState structure).
  */
-int Curl_resolver_duphandle(void **to, void *from)
+CURLcode Curl_resolver_duphandle(struct Curl_easy *easy, void **to, void *from)
 {
   (void)from;
-  return Curl_resolver_init(to);
+  return Curl_resolver_init(easy, to);
 }
 
 static void destroy_async_data(struct Curl_async *);

diff --git a/lib/asyn.h b/lib/asyn.h
index 3adc3664a29190e63b2a187d8aa1b6b7dfd6f6b3..43625bc3beb670dafc8e335372c3f3ae121a9efb 100644 (file)
--- a/lib/asyn.h
+++ b/lib/asyn.h
@@ -60,7 +60,7 @@ void Curl_resolver_global_cleanup(void);
  * Returning anything else than CURLE_OK fails curl_easy_init() with the
  * correspondent code.
  */
-CURLcode Curl_resolver_init(void **resolver);
+CURLcode Curl_resolver_init(struct Curl_easy *easy, void **resolver);
 
 /*
  * Curl_resolver_cleanup()
@@ -79,7 +79,8 @@ void Curl_resolver_cleanup(void *resolver);
  * pointer.  Returning anything else than CURLE_OK causes failed
  * curl_easy_duphandle() call.
  */
-int Curl_resolver_duphandle(void **to, void *from);
+CURLcode Curl_resolver_duphandle(struct Curl_easy *easy, void **to,
+                                 void *from);
 
 /*
  * Curl_resolver_cancel().
@@ -150,8 +151,8 @@ Curl_addrinfo *Curl_resolver_getaddrinfo(struct connectdata *conn,
 #define Curl_resolver_is_resolved(x,y) CURLE_COULDNT_RESOLVE_HOST
 #define Curl_resolver_wait_resolv(x,y) CURLE_COULDNT_RESOLVE_HOST
 #define Curl_resolver_getsock(x,y,z) 0
-#define Curl_resolver_duphandle(x,y) CURLE_OK
-#define Curl_resolver_init(x) CURLE_OK
+#define Curl_resolver_duphandle(x,y,z) CURLE_OK
+#define Curl_resolver_init(x,y) CURLE_OK
 #define Curl_resolver_global_init() CURLE_OK
 #define Curl_resolver_global_cleanup() Curl_nop_stmt
 #define Curl_resolver_cleanup(x) Curl_nop_stmt

diff --git a/lib/connect.c b/lib/connect.c
index 41f2202681f841a292496d31600fa800f79bc647..18645b9b7b33482c2862a23cf5e2dfc37d342639 100644 (file)
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -1314,7 +1314,7 @@ int Curl_closesocket(struct connectdata *conn,
       conn->sock_accepted[SECONDARYSOCKET] = FALSE;
     else {
       int rc;
-      Curl_multi_closed(conn, sock);
+      Curl_multi_closed(conn->data, sock);
       Curl_set_in_callback(conn->data, true);
       rc = conn->fclosesocket(conn->closesocket_client, sock);
       Curl_set_in_callback(conn->data, false);
@@ -1324,7 +1324,7 @@ int Curl_closesocket(struct connectdata *conn,
 
   if(conn)
     /* tell the multi-socket code about this */
-    Curl_multi_closed(conn, sock);
+    Curl_multi_closed(conn->data, sock);
 
   sclose(sock);
 

diff --git a/lib/easy.c b/lib/easy.c
index 4de4e6522f29abb251c1bde07f6ba266fda66b9e..d940b5d3e901cfd590d145e93df54b2263eee1c1 100644 (file)
--- a/lib/easy.c
+++ b/lib/easy.c
@@ -966,7 +966,8 @@ struct Curl_easy *curl_easy_duphandle(struct Curl_easy *data)
   }
 
   /* Clone the resolver handle, if present, for the new handle */
-  if(Curl_resolver_duphandle(&outcurl->state.resolver,
+  if(Curl_resolver_duphandle(outcurl,
+                             &outcurl->state.resolver,
                              data->state.resolver))
     goto fail;
 

diff --git a/lib/multi.c b/lib/multi.c
index 0db2a97308c64fede76b372a450ab1a363857177..fbd38971664d5bcae0ef59899aacc488f5de3230 100644 (file)
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -2466,11 +2466,11 @@ void Curl_updatesocket(struct Curl_easy *data)
  * socket again and it gets the same file descriptor number.
  */
 
-void Curl_multi_closed(struct connectdata *conn, curl_socket_t s)
+void Curl_multi_closed(struct Curl_easy *data, curl_socket_t s)
 {
-  if(conn->data) {
+  if(data) {
     /* if there's still an easy handle associated with this connection */
-    struct Curl_multi *multi = conn->data->multi;
+    struct Curl_multi *multi = data->multi;
     if(multi) {
       /* this is set if this connection is part of a handle that is added to
          a multi handle, and only then this is necessary */
@@ -2478,7 +2478,7 @@ void Curl_multi_closed(struct connectdata *conn, curl_socket_t s)
 
       if(entry) {
         if(multi->socket_cb)
-          multi->socket_cb(conn->data, s, CURL_POLL_REMOVE,
+          multi->socket_cb(data, s, CURL_POLL_REMOVE,
                            multi->socket_userp,
                            entry->socketp);
 

diff --git a/lib/multiif.h b/lib/multiif.h
index c8fb5ca0d1c6a1e0109df90d5b60649c770af866..c50063dae7de50a0a6a3d581d18ec6f28d78b44f 100644 (file)
--- a/lib/multiif.h
+++ b/lib/multiif.h
@@ -89,7 +89,7 @@ void Curl_multi_connchanged(struct Curl_multi *multi);
  * socket again and it gets the same file descriptor number.
  */
 
-void Curl_multi_closed(struct connectdata *conn, curl_socket_t s);
+void Curl_multi_closed(struct Curl_easy *data, curl_socket_t s);
 
 /*
  * Add a handle and move it into PERFORM state at once. For pushed streams.

diff --git a/lib/url.c b/lib/url.c
index 6118177d697ba3083ecaa04153425868d53be912..0ebf0a07a6145df2e15c843a2ab6fdb0acece1d1 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -570,7 +570,7 @@ CURLcode Curl_open(struct Curl_easy **curl)
 
   data->magic = CURLEASY_MAGIC_NUMBER;
 
-  result = Curl_resolver_init(&data->state.resolver);
+  result = Curl_resolver_init(data, &data->state.resolver);
   if(result) {
     DEBUGF(fprintf(stderr, "Error: resolver_init failed\n"));
     free(data);