Async DNS for OSX

Async DNS for OSX

[Magnus Henoch]

[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]

Hi all,

Since Emacs gained the capability to make asynchronous DNS lookups 
under GNU/Linux, I've been wishing for the same thing under OSX. 
Here is a little patch that I've hacked together, that I'd like 
some feedback on.  In particular:

1. In Fmake_network_process, there's an XXX comment about 
deallocating my CFStringRef.  I couldn't figure out from the 
documentation whether CFHostCreateWithName takes ownership of the 
string, or just copies it.

2. There is a callback function, host_client_callback, that writes 
error information into memory referred to by the process struct. 
There is a potential for a race condition here: what if the 
callback is called after the process has been deallocated?  I've 
tried to avoid that by cancelling name resolution before 
deallocating the memory (see free_dns_request), but I suspect 
that's not enough.  Is there a clever way to avoid that?

3. Unlike getaddrinfo_a, this CFHost thing doesn't resolve service 
names to port numbers.  Therefore, I arrange to call getservbyname 
in Fmake_network_process, and insert the port number into a copy 
of the sockaddr struct in check_for_dns.  I _think_ this should 
work for both IPv4 and IPv6, but I haven't tested IPv6.  Is there 
a nicer way to do this?

It took me a while to figure out the port number thing: before I 
added the port number, the process seemed to silently do nothing. 
That's because non-blocking connect would fail, and 
connect_network_socket would treat that as having to do a 
synchronous connect, which ended up not happening.  I'm not sure 
if this is a bug or a feature.

Any other comments are appreciated as well.  I'll have sporadic 
Internet access during the coming week or so, will probably look 
at this again after that time.

Regards,
Magnus



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: async-dns-osx.patch --]
[-- Type: text/x-patch, Size: 10494 bytes --]

diff --git a/configure.ac b/configure.ac
index 5a6a72a..94c2947 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2436,6 +2436,12 @@ if test "${HAVE_GETADDRINFO_A}" = "yes"; then
   AC_SUBST(GETADDRINFO_A_LIBS)
 fi
 
+AC_CHECK_HEADER(CFNetwork/CFHost.h,
+  dnl XXX: Do we need to test for existence here?
+  LIBS="$LIBS -framework CFNetwork"
+  AC_DEFINE(HAVE_CFHOSTCREATEWITHNAME, 1,
+[Define to 1 if you have CFHostCreateWithName for asynchronous DNS resolution.]))
+
 HAVE_GTK=no
 GTK_OBJ=
 gtk_term_header=$term_header
diff --git a/src/process.c b/src/process.c
index 2b674e5..de27c31 100644
--- a/src/process.c
+++ b/src/process.c
@@ -87,6 +87,10 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <sig2str.h>
 #include <verify.h>
 
+#ifdef HAVE_CFHOSTCREATEWITHNAME
+#include <CFNetwork/CFHost.h>
+#endif
+
 #endif	/* subprocesses */
 
 #include "systime.h"
@@ -125,6 +129,10 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #define ASYNC_RETRY_NSEC 100000000
 #endif
 
+#ifdef HAVE_CFHOSTCREATEWITHNAME
+static void host_client_callback (CFHostRef, CFHostInfoType, const CFStreamError *, void *);
+#endif
+
 #ifdef WINDOWSNT
 extern int sys_select (int, fd_set *, fd_set *, fd_set *,
 		       struct timespec *, void *);
@@ -733,15 +741,24 @@ remove_process (register Lisp_Object proc)
   deactivate_process (proc);
 }
 
-#ifdef HAVE_GETADDRINFO_A
+#if defined HAVE_GETADDRINFO_A || defined HAVE_CFHOSTCREATEWITHNAME
 static void
 free_dns_request (Lisp_Object proc)
 {
   struct Lisp_Process *p = XPROCESS (proc);
 
+#ifdef HAVE_GETADDRINFO_A
   if (p->dns_request->ar_result)
     freeaddrinfo (p->dns_request->ar_result);
   xfree (p->dns_request);
+#elif defined HAVE_CFHOSTCREATEWITHNAME
+  CFRunLoopRef run_loop = CFRunLoopGetCurrent ();
+  CFHostUnscheduleFromRunLoop (p->dns_request, run_loop, kCFRunLoopDefaultMode);
+  CFHostCancelInfoResolution (p->dns_request, kCFHostAddresses);
+  CFRelease (p->dns_request);
+  xfree (p->dns_error);
+  p->dns_error = NULL;
+#endif
   p->dns_request = NULL;
 }
 #endif
@@ -853,6 +870,10 @@ nil, indicating the current buffer's process.  */)
       if (canceled)
 	free_dns_request (process);
     }
+#elif defined HAVE_CFHOSTCREATEWITHNAME
+  if (p->dns_request)
+    /* free_dns_request will cancel the request for us. */
+    free_dns_request (process);
 #endif
 
   p->raw_status_new = 0;
@@ -3611,6 +3632,9 @@ usage: (make-network-process &rest ARGS)  */)
   int ai_protocol = 0;
 #ifdef HAVE_GETADDRINFO_A
   struct gaicb *dns_request = NULL;
+#elif defined HAVE_CFHOSTCREATEWITHNAME
+  CFHostRef dns_request = NULL;
+  CFStreamError *dns_error = NULL;
 #endif
   ptrdiff_t count = SPECPDL_INDEX ();
 
@@ -3791,7 +3815,41 @@ usage: (make-network-process &rest ARGS)  */)
 
       goto open_socket;
     }
-#endif /* HAVE_GETADDRINFO_A */
+#elif defined(HAVE_CFHOSTCREATEWITHNAME)
+  if (!NILP (host) && !NILP (Fplist_get (contact, QCnowait)))
+    {
+      CFStringRef stringref;
+
+      stringref = CFStringCreateWithCString (NULL, SSDATA (host),
+					     kCFStringEncodingASCII);
+      if (! stringref)
+	error ("%s CFStringCreateWithCString error", SSDATA (host));
+      /* XXX: when should I deallocate stringref? */
+      dns_request = CFHostCreateWithName (NULL, stringref);
+      if (! dns_request)
+	error ("%s CFHostCreateWithName error", SSDATA (host));
+
+      dns_error = xmalloc (sizeof *dns_error);
+      memset (dns_error, 0, sizeof *dns_error);
+      CFHostClientContext client_context;
+      memset (&client_context, 0, sizeof client_context);
+      client_context.version = 0;
+      client_context.info = (void*)dns_error;
+      if (! CFHostSetClient (dns_request, host_client_callback, &client_context))
+	error ("%s CFHostSetClient error", SSDATA (host));
+
+      CFRunLoopRef run_loop = CFRunLoopGetCurrent ();
+      CFHostScheduleWithRunLoop (dns_request, run_loop, kCFRunLoopDefaultMode);
+
+      /* TODO: need error information? */
+      if (! CFHostStartInfoResolution (dns_request, kCFHostAddresses, NULL))
+	error ("%s CFHostStartInfoResolution error", SSDATA (host));
+
+      /* Unlike getaddrinfo_a, this doesn't resolve the service name
+	 for us. */
+      goto resolve_service;
+    }
+#endif /* HAVE_GETADDRINFO_A | HAVE_CFHOSTCREATEWITHNAME */
 
   /* If we have a host, use getaddrinfo to resolve both host and service.
      Otherwise, use getservbyname to lookup the service.  */
@@ -3842,6 +3900,8 @@ usage: (make-network-process &rest ARGS)  */)
 
   /* No hostname has been specified (e.g., a local server process).  */
 
+ resolve_service:
+
   if (EQ (service, Qt))
     port = 0;
   else if (INTEGERP (service))
@@ -3903,6 +3963,9 @@ usage: (make-network-process &rest ARGS)  */)
   p->ai_protocol = ai_protocol;
 #ifdef HAVE_GETADDRINFO_A
   p->dns_request = NULL;
+#elif defined HAVE_CFHOSTCREATEWITHNAME
+  p->dns_request = NULL;
+  p->dns_error = NULL;
 #endif
 #ifdef HAVE_GNUTLS
   tem = Fplist_get (contact, QCtls_parameters);
@@ -3930,12 +3993,15 @@ usage: (make-network-process &rest ARGS)  */)
       && !NILP (Fplist_get (contact, QCnowait)))
     p->is_non_blocking_client = true;
 
-#ifdef HAVE_GETADDRINFO_A
+#if defined HAVE_GETADDRINFO_A || defined HAVE_CFHOSTCREATEWITHNAME
   /* With async address resolution, the list of addresses is empty, so
      postpone connecting to the server. */
   if (!p->is_server && NILP (ip_addresses))
     {
       p->dns_request = dns_request;
+#ifdef HAVE_CFHOSTCREATEWITHNAME
+      p->dns_error = dns_error;
+#endif
       p->status = Qconnect;
       return proc;
     }
@@ -4644,7 +4710,21 @@ server_accept_connection (Lisp_Object server, int channel)
   exec_sentinel (proc, concat3 (open_from, host_string, nl));
 }
 
-#ifdef HAVE_GETADDRINFO_A
+#ifdef HAVE_CFHOSTCREATEWITHNAME
+static void
+host_client_callback (CFHostRef theHost, CFHostInfoType typeInfo, const CFStreamError *error, void *info)
+{
+  if (error)
+    {
+      CFStreamError *dns_error = (CFStreamError *)info;
+      /* XXX: how can I know that the process hasn't been deallocated? */
+      dns_error->domain = error->domain;
+      dns_error->error = error->error;
+    }
+}
+#endif
+
+#if defined HAVE_GETADDRINFO_A || defined HAVE_CFHOSTCREATEWITHNAME
 static Lisp_Object
 check_for_dns (Lisp_Object proc)
 {
@@ -4655,6 +4735,7 @@ check_for_dns (Lisp_Object proc)
   if (! p->dns_request)
     return Qnil;
 
+#ifdef HAVE_GETADDRINFO_A
   int ret = gai_error (p->dns_request);
   if (ret == EAI_INPROGRESS)
     return Qt;
@@ -4683,6 +4764,55 @@ check_for_dns (Lisp_Object proc)
 				 build_string (p->dns_request->ar_name),
 				 build_string (" failed")))));
     }
+#elif defined HAVE_CFHOSTCREATEWITHNAME
+
+  Boolean has_been_resolved;
+  CFArrayRef res;
+
+  res = CFHostGetAddressing (p->dns_request, &has_been_resolved);
+
+  /* XXX: if has_been_resolved is false, does that mean there was an error? */
+  if (has_been_resolved && res)
+    {
+      CFIndex n;
+      n = CFArrayGetCount (res);
+      for (CFIndex i = 0; i < n; i++)
+	{
+	  CFDataRef entry = CFArrayGetValueAtIndex (res, i);
+	  const struct sockaddr *addr = (const struct sockaddr *) CFDataGetBytePtr (entry);
+	  CFIndex len = CFDataGetLength (entry);
+	  /* Need to insert the port number. */
+	  struct sockaddr *addr_with_port = xmalloc (len);
+	  memcpy (addr_with_port, addr, len);
+	  /* XXX: this should work for IPv4 and IPv6, I think */
+	  ((struct sockaddr_in*)addr_with_port)->sin_port = htons (p->port);
+	  ip_addresses = Fcons (conv_sockaddr_to_lisp
+				(addr_with_port, len),
+				ip_addresses);
+	  xfree (addr_with_port);
+
+	  ip_addresses = Fnreverse (ip_addresses);
+	}
+    }
+  else if (p->dns_error->domain)
+    {
+      add_to_log ("async DNS resulution error"); /* XXX */
+      deactivate_process (proc);
+      pset_status (p, (list2
+		       (Qfailed,
+			concat2 (
+				 concat2 (build_string ("Name lookup failed; error domain "),
+					  Fnumber_to_string (make_number (p->dns_error->domain))),
+				 concat2 (build_string (", error number "),
+					  Fnumber_to_string (make_number (p->dns_error->error)))))));
+    }
+  else
+    {
+      /* Still in progress ??? */
+      add_to_log ("Name resolution still in progress"); /* XXX */
+      return Qt;
+    }
+#endif
 
   free_dns_request (proc);
 
@@ -4693,7 +4823,7 @@ check_for_dns (Lisp_Object proc)
   return ip_addresses;
 }
 
-#endif /* HAVE_GETADDRINFO_A */
+#endif /* HAVE_GETADDRINFO_A || HAVE_CFHOSTCREATEWITHNAME */
 
 static void
 wait_for_socket_fds (Lisp_Object process, char const *name)
@@ -4805,9 +4935,10 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
   Lisp_Object proc;
   struct timespec timeout, end_time, timer_delay;
   struct timespec got_output_end_time = invalid_timespec ();
+#undef INFINITY
   enum { MINIMUM = -1, TIMEOUT, INFINITY } wait;
   int got_some_output = -1;
-#if defined HAVE_GETADDRINFO_A || defined HAVE_GNUTLS
+#if defined HAVE_GETADDRINFO_A || defined HAVE_CFHOSTCREATEWITHNAME || defined HAVE_GNUTLS
   bool retry_for_async;
 #endif
   ptrdiff_t count = SPECPDL_INDEX ();
@@ -4857,7 +4988,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
       if (! NILP (wait_for_cell) && ! NILP (XCAR (wait_for_cell)))
 	break;
 
-#if defined HAVE_GETADDRINFO_A || defined HAVE_GNUTLS
+#if defined HAVE_GETADDRINFO_A || defined HAVE_CFHOSTCREATEWITHNAME || defined HAVE_GNUTLS
       {
 	Lisp_Object process_list_head, aproc;
 	struct Lisp_Process *p;
@@ -4869,7 +5000,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 
 	    if (! wait_proc || p == wait_proc)
 	      {
-#ifdef HAVE_GETADDRINFO_A
+#if defined HAVE_GETADDRINFO_A || defined HAVE_CFHOSTCREATEWITHNAME
 		/* Check for pending DNS requests. */
 		if (p->dns_request)
 		  {
diff --git a/src/process.h b/src/process.h
index bf1eadc..e47a53e 100644
--- a/src/process.h
+++ b/src/process.h
@@ -25,6 +25,10 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <unistd.h>
 
+#ifdef HAVE_CFHOSTCREATEWITHNAME
+#include <CFNetwork/CFHost.h>
+#endif
+
 #ifdef HAVE_GNUTLS
 #include "gnutls.h"
 #endif
@@ -180,6 +184,9 @@ struct Lisp_Process
     /* Whether the socket is waiting for response from an asynchronous
        DNS call. */
     struct gaicb *dns_request;
+#elif defined (HAVE_CFHOSTCREATEWITHNAME)
+    CFHostRef dns_request;
+    CFStreamError *dns_error;
 #endif
 
 #ifdef HAVE_GNUTLS

Re: Async DNS for OSX

[Paul Eggert]

Would it make sense to do this as a gnulib module for getaddrinfo_a? 
That way, Emacs could just continue to use the GNU/Linux interface, and 
other programs needing async DNS could do so as well, without too much 
trouble.

Re: Async DNS for OSX

[Lars Ingebrigtsen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Would it make sense to do this as a gnulib module for getaddrinfo_a?
> That way, Emacs could just continue to use the GNU/Linux interface,
> and other programs needing async DNS could do so as well, without too
> much trouble.

It that were possible, it would be much cleaner (and as you say, more
generally useful).

Implementing the full getaddrinfo_a interface would be a bigger job than
just the simple bits that Emacs uses (signal handling, etc), though.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Async DNS for OSX

[Paul Eggert]

On 05/25/2016 11:34 AM, Lars Ingebrigtsen wrote:
> Implementing the full getaddrinfo_a interface would be a bigger job than
> just the simple bits that Emacs uses (signal handling, etc), though.

If the Gnulib module supports just the part of getaddrinfo_a that Emacs 
needs, that should be good enough. Lots of Gnulib modules implement only 
the "important" parts of the portable interface, where "important" means 
"needed by GNU apps that use Gnulib", a definition that can change with 
time as usage grows.

If all this is too much trouble, then just patching Emacs is OK too. We 
can worry about libraryizing a getaddrinfo_a substitute later.

Re: Async DNS for OSX

[Lars Ingebrigtsen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> If the Gnulib module supports just the part of getaddrinfo_a that
> Emacs needs, that should be good enough. Lots of Gnulib modules
> implement only the "important" parts of the portable interface, where
> "important" means "needed by GNU apps that use Gnulib", a definition
> that can change with time as usage grows.

Cool.

> If all this is too much trouble, then just patching Emacs is OK
> too. We can worry about libraryizing a getaddrinfo_a substitute later.

My guess is that it would be easier for people to implement a partial
getaddrinfo_a in a Gnulib context than to implement async DNS in an
Emacs context.  I think.  Does Gnulib have getaddrinfo_a scaffolding
done?  (I'm assuming there must be something of the kind per function it
implements.)  Perhaps if that were in place (and included in Emacs),
then it would be even simpler for people to implement the missing OS
X/Windows bits?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Async DNS for OSX

[Paul Eggert]

On 05/26/2016 03:46 AM, Lars Ingebrigtsen wrote:
> Does Gnulib have getaddrinfo_a scaffolding
> done?  (I'm assuming there must be something of the kind per function it
> implements.)
No, unfortunately.

> Perhaps if that were in place (and included in Emacs),
> then it would be even simpler for people to implement the missing OS
> X/Windows bits?

Yes, that's the idea.