Re: Asynchronous DNS

[Alain Schneble <a.s@realize.ch>]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> The issue is losing asynchronicity.  A function that previously called
>
> (progn
>   (make-network-stream ... :nowait t)
>   (set-process-coding-system ...))
>
> would not block.  With the proposed blockers (unless we add fine-grained
> code to all the process function to only block if we haven't even done
> DNS yet), this code will block.  And that's a regression.

By the way, I tried it out with the fine-grained approach.  See patch
below.  The majority of the functions touched will block if DNS requests
are still pending.  process-send-* functions block while process is in
"connect" state.  I didn't have time to verify it with TLS connections
though, and also just very quickly with http connections and some hand
crafted scenarios...

> But, I mean, we could examine all these process functions and only have
> them block where we need them to, and things would work fine.  I don't
> really have much confidence in us being able to do so completely
> transparently, though.

There is potential to refactor at least two functions,
set-process-coding-system and set-process-window-size, to not require
blocking at all.  The former will require adaptions in
set_network_socket_coding_system, to check whether coding systems have
already been set, so that they do not get overwritten, e.g. in this
scenario:

(progn
  (make-network-stream ... :nowait t)
  (set-process-coding-system ...))

And set-process-coding-system could just jump over the call to
setup_process_coding_systems in case of a network process with pending
DNS requests (i.e. socket not yet available).

While with set-process-window-size, I asked myself if this one makes any
sense with network processes at all.  Should it signal an error or what
would be the use case to call this for a network process?

And here is the small patch I used:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch -- Make-process-functions-wait-for-DNS-completion --]
[-- Type: text/x-patch, Size: 6439 bytes --]

From 1ceb2e58aa1a7d688439aa9e4c0ed11fb0e50534 Mon Sep 17 00:00:00 2001
From: Alain Schneble <a.s@realize.ch>
Date: Mon, 15 Feb 2016 00:11:06 +0100
Subject: [PATCH] Make process functions wait for DNS completion

* src/process.c (set-process-filter, set-process-window-size,
process-contact, process-datagram-address, set-process-datagram-address,
set-network-process-option): make functions wait (block) on network
process until pending DNS requests have been processed and associated
socket initialized.

* src/process.c (process-send-region, process-send-string,
process-send-eof): make functions wait (block) while network process is
in connect state.
---
 src/process.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 84 insertions(+), 7 deletions(-)

diff --git a/src/process.c b/src/process.c
index 497b069..9816e93 100644
--- a/src/process.c
+++ b/src/process.c
@@ -284,6 +284,7 @@ static Lisp_Object chan_process[FD_SETSIZE];
 #ifdef HAVE_GETADDRINFO_A
 /* Pending DNS requests. */
 static Lisp_Object dns_processes;
+static void wait_for_socket_fds (Lisp_Object process);
 #endif
 
 /* Alist of elements (NAME . PROCESS).  */
@@ -1029,6 +1030,12 @@ The string argument is normally a multibyte string, except:
   struct Lisp_Process *p;
 
   CHECK_PROCESS (process);
+
+#ifdef HAVE_GETADDRINFO_A
+  if (NETCONN_P (process))
+    wait_for_socket_fds (process);
+#endif
+
   p = XPROCESS (process);
 
   /* Don't signal an error if the process's input file descriptor
@@ -1113,6 +1120,11 @@ DEFUN ("set-process-window-size", Fset_process_window_size,
 {
   CHECK_PROCESS (process);
 
+#ifdef HAVE_GETADDRINFO_A
+  if (NETCONN_P (process))
+    wait_for_socket_fds (process);
+#endif
+
   /* All known platforms store window sizes as 'unsigned short'.  */
   CHECK_RANGED_INTEGER (height, 0, USHRT_MAX);
   CHECK_RANGED_INTEGER (width, 0, USHRT_MAX);
@@ -1194,6 +1206,12 @@ list of keywords.  */)
   contact = XPROCESS (process)->childp;
 
 #ifdef DATAGRAM_SOCKETS
+
+#ifdef HAVE_GETADDRINFO_A
+  if (NETCONN_P (process))
+    wait_for_socket_fds (process);
+#endif
+
   if (DATAGRAM_CONN_P (process)
       && (EQ (key, Qt) || EQ (key, QCremote)))
     contact = Fplist_put (contact, QCremote,
@@ -2423,6 +2441,11 @@ DEFUN ("process-datagram-address", Fprocess_datagram_address, Sprocess_datagram_
 
   CHECK_PROCESS (process);
 
+#ifdef HAVE_GETADDRINFO_A
+  if (NETCONN_P (process))
+    wait_for_socket_fds (process);
+#endif
+
   if (!DATAGRAM_CONN_P (process))
     return Qnil;
 
@@ -2442,6 +2465,11 @@ Returns nil upon error setting address, ADDRESS otherwise.  */)
 
   CHECK_PROCESS (process);
 
+#ifdef HAVE_GETADDRINFO_A
+  if (NETCONN_P (process))
+    wait_for_socket_fds (process);
+#endif
+
   if (!DATAGRAM_CONN_P (process))
     return Qnil;
 
@@ -2610,6 +2638,10 @@ OPTION is not a supported option, return nil instead; otherwise return t.  */)
   if (!NETCONN1_P (p))
     error ("Process is not a network process");
 
+#ifdef HAVE_GETADDRINFO_A
+  wait_for_socket_fds (process);
+#endif
+
   s = p->infd;
   if (s < 0)
     error ("Process is not running");
@@ -3693,7 +3725,7 @@ usage: (make-network-process &rest ARGS)  */)
 #endif
 
 #ifdef HAVE_GETADDRINFO_A
-  if (EQ (Fplist_get (contact, QCnowait), Qdns) &&
+  if (EQ (Fplist_get (contact, QCnowait), Qt) &&
       !NILP (host))
     {
       int ret;
@@ -4650,6 +4682,24 @@ check_for_dns (Lisp_Object proc)
 
   return ip_addresses;
 }
+
+static void
+wait_for_socket_fds(Lisp_Object process)
+{
+  while (XPROCESS(process)->dns_requests)
+    {
+      wait_reading_process_output (0, 20 * 1000 * 1000, 0, 0, Qnil, NULL, 0);
+    }
+}
+
+static void
+wait_while_connecting(Lisp_Object process)
+{
+  while (EQ (Qconnect, XPROCESS(process)->status))
+    {
+      wait_reading_process_output (0, 20 * 1000 * 1000, 0, 0, Qnil, NULL, 0);
+    }
+}
 #endif /* HAVE_GETADDRINFO_A */
 
 /* This variable is different from waiting_for_input in keyboard.c.
@@ -6143,6 +6193,11 @@ Output from processes can arrive in between bunches.  */)
   if (XINT (start) < GPT && XINT (end) > GPT)
     move_gap_both (XINT (start), start_byte);
 
+#ifdef HAVE_GETADDRINFO_A
+  if (NETCONN_P (proc))
+    wait_while_connecting (proc);
+#endif
+  
   send_process (proc, (char *) BYTE_POS_ADDR (start_byte),
 		end_byte - start_byte, Fcurrent_buffer ());
 
@@ -6162,6 +6217,12 @@ Output from processes can arrive in between bunches.  */)
   Lisp_Object proc;
   CHECK_STRING (string);
   proc = get_process (process);
+
+#ifdef HAVE_GETADDRINFO_A
+  if (NETCONN_P (proc))
+    wait_while_connecting (proc);
+#endif
+
   send_process (proc, SSDATA (string),
 		SBYTES (string), string);
   return Qnil;
@@ -6576,10 +6637,17 @@ process has been transmitted to the serial port.  */)
   struct coding_system *coding = NULL;
   int outfd;
 
-  if (DATAGRAM_CONN_P (process))
+  proc = get_process (process);
+
+#ifdef HAVE_GETADDRINFO_A
+  if (NETCONN_P (proc))
+    wait_while_connecting (proc);
+#endif
+
+  if (DATAGRAM_CONN_P (proc))
     return process;
 
-  proc = get_process (process);
+  
   outfd = XPROCESS (proc)->outfd;
   if (outfd >= 0)
     coding = proc_encode_coding_system[outfd];
@@ -7030,7 +7098,14 @@ encode subprocess input.  */)
   register struct Lisp_Process *p;
 
   CHECK_PROCESS (process);
+
+#ifdef HAVE_GETADDRINFO_A
+  if (NETCONN_P (process))
+    wait_for_socket_fds (process);
+#endif
+
   p = XPROCESS (process);
+  
   if (p->infd < 0)
     error ("Input file descriptor of %s closed", SDATA (p->name));
   if (p->outfd < 0)
@@ -7067,6 +7142,12 @@ suppressed.  */)
   register struct Lisp_Process *p;
 
   CHECK_PROCESS (process);
+
+#ifdef HAVE_GETADDRINFO_A
+  if (NETCONN_P (process))
+    wait_for_socket_fds (process);
+#endif
+
   p = XPROCESS (process);
   if (NILP (flag))
     pset_decode_coding_system
@@ -7757,7 +7838,6 @@ syms_of_process (void)
   DEFSYM (QCcoding, ":coding");
   DEFSYM (QCserver, ":server");
   DEFSYM (QCnowait, ":nowait");
-  DEFSYM (Qdns, "dns");
   DEFSYM (QCsentinel, ":sentinel");
   DEFSYM (QCtls_parameters, ":tls-parameters");
   DEFSYM (QClog, ":log");
@@ -7921,9 +8001,6 @@ The variable takes effect when `start-process' is called.  */);
 
 #ifdef NON_BLOCKING_CONNECT
    ADD_SUBFEATURE (QCnowait, Qt);
-#ifdef HAVE_GETADDRINFO_A
-   ADD_SUBFEATURE (QCnowait, Qdns);
-#endif
 #endif
 #ifdef DATAGRAM_SOCKETS
    ADD_SUBFEATURE (QCtype, Qdatagram);
-- 
2.6.2.windows.1