Subject: bug#23982: closed (Re: bug#23982: [master] Treat errno EINPROGRESS and ENOTCONN as EAGAIN for async connection)

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 03 Aug 2016 09:03:02 +0000

Your bug report

#23982: [master] Treat errno EINPROGRESS and ENOTCONN as EAGAIN for async connection

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
23982: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23982
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#23982: [master] Treat errno EINPROGRESS and ENOTCONN as EAGAIN for async connection
From: Paul Eggert <[email-masked]>
To: Eli Zaretskii <[email-masked]>, Ted Zlatanov <[email-masked]>
Cc: [email-masked], [email-masked], [email-masked]
Date: Wed, 3 Aug 2016 02:02:11 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Eli Zaretskii wrote:
> I'm building Emacs with GnuTLS 3.3.11 since many moons ago.

OK, I installed the attached patches to GNU Emacs master. The first one requires 
GnuTLS 2.12.2 or later. The second one uses the GNUTLS_NONBLOCK flag, and works 
around the apparent GnuTLS bug with EINPROGRESS and ENOTCONN.

From b283c603b546f678c3eb7d31b06c28b7da285962 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Wed, 3 Aug 2016 04:33:21 -0400
Subject: [PATCH 1/2] Require GnuTLS 2.12.2 or later
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* configure.ac, etc/NEWS: Require GnuTLS 2.12.2 or later,
instead of merely 2.6.6 or later (Bug#23982#20).
* src/gnutls.c (init_gnutls_functions, emacs_gnutls_handshake)
[WINDOWSNT]: Don’t bother with gnutls_check_version or
gnutls_transport_set_lowat, as they are no longer needed now that
we require gnutls 2.12.2 or later.
---
 configure.ac |  2 +-
 etc/NEWS     |  3 +++
 src/gnutls.c | 23 -----------------------
 3 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/configure.ac b/configure.ac
index cca555c..e8aeae2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2748,7 +2748,7 @@ AC_DEFUN
 
 HAVE_GNUTLS=no
 if test "${with_gnutls}" = "yes" ; then
-  EMACS_CHECK_MODULES([LIBGNUTLS], [gnutls >= 2.6.6],
+  EMACS_CHECK_MODULES([LIBGNUTLS], [gnutls >= 2.12.2],
     [HAVE_GNUTLS=yes], [HAVE_GNUTLS=no])
   if test "${HAVE_GNUTLS}" = "yes"; then
     AC_DEFINE(HAVE_GNUTLS, 1, [Define if using GnuTLS.])
diff --git a/etc/NEWS b/etc/NEWS
index 6462eff..fadf4c2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -28,6 +28,9 @@ otherwise leave it unmarked.
 ** By default libgnutls is now required when building Emacs.
 Use 'configure --with-gnutls=no' to build even when GnuTLS is missing.
 
+** GnuTLS version 2.12.2 or later is now required, instead of merely
+version 2.6.6 or later.
+
 ** The new option 'configure --enable-gcc-warnings=warn-only' causes
 GCC to issue warnings without stopping the build.  This behavior is
 now the default in developer builds.  As before, use
diff --git a/src/gnutls.c b/src/gnutls.c
index 7f05ac4..681e298 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -110,8 +110,6 @@ DEF_DLL_FN (ssize_t, gnutls_record_send,
 	    (gnutls_session_t, const void *, size_t));
 DEF_DLL_FN (const char *, gnutls_strerror, (int));
 DEF_DLL_FN (void, gnutls_transport_set_errno, (gnutls_session_t, int));
-DEF_DLL_FN (const char *, gnutls_check_version, (const char *));
-DEF_DLL_FN (void, gnutls_transport_set_lowat, (gnutls_session_t, int));
 DEF_DLL_FN (void, gnutls_transport_set_ptr2,
 	    (gnutls_session_t, gnutls_transport_ptr_t,
 	     gnutls_transport_ptr_t));
@@ -225,11 +223,6 @@ init_gnutls_functions (void)
   LOAD_DLL_FN (library, gnutls_record_send);
   LOAD_DLL_FN (library, gnutls_strerror);
   LOAD_DLL_FN (library, gnutls_transport_set_errno);
-  LOAD_DLL_FN (library, gnutls_check_version);
-  /* We don't need to call gnutls_transport_set_lowat in GnuTLS 2.11.1
-     and later, and the function was removed entirely in 3.0.0.  */
-  if (!fn_gnutls_check_version ("2.11.1"))
-    LOAD_DLL_FN (library, gnutls_transport_set_lowat);
   LOAD_DLL_FN (library, gnutls_transport_set_ptr2);
   LOAD_DLL_FN (library, gnutls_transport_set_pull_function);
   LOAD_DLL_FN (library, gnutls_transport_set_push_function);
@@ -290,7 +283,6 @@ init_gnutls_functions (void)
 # define gnutls_certificate_set_x509_trust_file fn_gnutls_certificate_set_x509_trust_file
 # define gnutls_certificate_type_get fn_gnutls_certificate_type_get
 # define gnutls_certificate_verify_peers2 fn_gnutls_certificate_verify_peers2
-# define gnutls_check_version fn_gnutls_check_version
 # define gnutls_cipher_get fn_gnutls_cipher_get
 # define gnutls_cipher_get_name fn_gnutls_cipher_get_name
 # define gnutls_credentials_set fn_gnutls_credentials_set
@@ -321,7 +313,6 @@ init_gnutls_functions (void)
 # define gnutls_sign_get_name fn_gnutls_sign_get_name
 # define gnutls_strerror fn_gnutls_strerror
 # define gnutls_transport_set_errno fn_gnutls_transport_set_errno
-# define gnutls_transport_set_lowat fn_gnutls_transport_set_lowat
 # define gnutls_transport_set_ptr2 fn_gnutls_transport_set_ptr2
 # define gnutls_transport_set_pull_function fn_gnutls_transport_set_pull_function
 # define gnutls_transport_set_push_function fn_gnutls_transport_set_push_function
@@ -439,20 +430,6 @@ emacs_gnutls_handshake (struct Lisp_Process *proc)
 				 (gnutls_transport_ptr_t) proc);
       gnutls_transport_set_push_function (state, &emacs_gnutls_push);
       gnutls_transport_set_pull_function (state, &emacs_gnutls_pull);
-
-      /* For non blocking sockets or other custom made pull/push
-	 functions the gnutls_transport_set_lowat must be called, with
-	 a zero low water mark value. (GnuTLS 2.10.4 documentation)
-
-	 (Note: this is probably not strictly necessary as the lowat
-	  value is only used when no custom pull/push functions are
-	  set.)  */
-      /* According to GnuTLS NEWS file, lowat level has been set to
-	 zero by default in version 2.11.1, and the function
-	 gnutls_transport_set_lowat was removed from the library in
-	 version 2.99.0.  */
-      if (!gnutls_check_version ("2.11.1"))
-	gnutls_transport_set_lowat (state, 0);
 #else
       /* This is how GnuTLS takes sockets: as file descriptors passed
 	 in.  For an Emacs process socket, infd and outfd are the
-- 
2.5.5

From 3029f74ad5b8f989e26673a31c257fa2aa8d7304 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Wed, 3 Aug 2016 01:54:20 -0700
Subject: [PATCH 2/2] Fix non-blocking GnuTLS with slow connection

Although the problem is reported for OS X (Bug#23982), it seems to
be possible on other POSIXish platforms.
* src/gnutls.c (emacs_gnutls_nonblock_errno) [!WINDOWSNT]:
New function.
(emacs_gnutls_handshake) [!WINDOWSNT]:
Use it as the errno function, if non-blocking.
(Fgnutls_boot): Use GNUTLS_NONBLOCK if non-blocking.
---
 src/gnutls.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/src/gnutls.c b/src/gnutls.c
index 681e298..e3e9311 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -411,6 +411,31 @@ gnutls_try_handshake (struct Lisp_Process *proc)
   return ret;
 }
 
+#ifndef WINDOWSNT
+static int
+emacs_gnutls_nonblock_errno (gnutls_transport_ptr_t ptr)
+{
+  int err = errno;
+
+  switch (err)
+    {
+# ifdef _AIX
+      /* This is taken from the GnuTLS system_errno function circa 2016;
+	 see <http://savannah.gnu.org/support/?107464>.  */
+    case 0:
+      errno = EAGAIN;
+      /* Fall through.  */
+# endif
+    case EINPROGRESS:
+    case ENOTCONN:
+      return EAGAIN;
+
+    default:
+      return err;
+    }
+}
+#endif
+
 static int
 emacs_gnutls_handshake (struct Lisp_Process *proc)
 {
@@ -437,6 +462,9 @@ emacs_gnutls_handshake (struct Lisp_Process *proc)
       gnutls_transport_set_ptr2 (state,
 				 (void *) (intptr_t) proc->infd,
 				 (void *) (intptr_t) proc->outfd);
+      if (proc->is_non_blocking_client)
+	gnutls_transport_set_errno_function (state,
+					     emacs_gnutls_nonblock_errno);
 #endif
 
       proc->gnutls_initstage = GNUTLS_STAGE_TRANSPORT_POINTERS_SET;
@@ -1574,7 +1602,8 @@ one trustfile (usually a CA bundle).  */)
   /* Call gnutls_init here: */
 
   GNUTLS_LOG (1, max_log_level, "gnutls_init");
-  ret = gnutls_init (&state, GNUTLS_CLIENT);
+  int nonblock = XPROCESS (proc)->is_non_blocking_client ? GNUTLS_NONBLOCK : 0;
+  ret = gnutls_init (&state, GNUTLS_CLIENT | nonblock);
   XPROCESS (proc)->gnutls_state = state;
   if (ret < GNUTLS_E_SUCCESS)
     return gnutls_make_error (ret);
-- 
2.5.5

Subject: [master] Treat errno EINPROGRESS and ENOTCONN as EAGAIN for async connection
From: Jun Hao <[email-masked]>
To: [email-masked]
Date: Thu, 14 Jul 2016 22:17:13 +0800
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi,

For latest master branch, when try to use gnutls with async connection,
the handshake will return fatal error if socket is in ENOTCONN or
EINPROGRESS since gnutls treat these errors as fatal during push/pull
and gives up. The later retry will fail because gnutls will mark the
session invalid.

This patch is asking gnutls to treat them as EAGAIN which is non-fatal.

I only tested with OSX. Please see if you can reproduce it on Windows or
Linux and if this patch works for them too.

To reproduce run:

emacs -Q --eval "(erc-tls :server \"irc.freenode.net\" :port 6697 :nick \"test\")"

Current master branch will gave up connecting.

-- 
Thanks - Jun

>From 8c69cab078d4c51d5c8f76f2aacb7bb8dd46dd7f Mon Sep 17 00:00:00 2001
From: Jun Hao <[email-masked]>
Date: Thu, 14 Jul 2016 21:47:24 +0800
Subject: [PATCH] Treat errno EINPROGRESS and ENOTCONN as EAGAIN for async
 connection

* src/gnutls.c: (emacs_gnutls_non_blocking_errno): treat errno
EINPROGRESS and ENOTCONN as EAGAIN
(emacs_gnutls_handshake): set errno function to it when async
connection
(Fgnutls_boot): set state with GNUTLS_NONBLOCK flag when async
connection
---
 src/gnutls.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/src/gnutls.c b/src/gnutls.c
index 7f05ac4..449a971 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -35,6 +35,10 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
 static bool emacs_gnutls_handle_error (gnutls_session_t, int);
 
+#ifndef WINDOWSNT
+static int emacs_gnutls_non_blocking_errno(gnutls_transport_ptr_t ptr);
+#endif
+
 static bool gnutls_global_initialized;
 
 static void gnutls_log_function (int, const char *);
@@ -383,6 +387,21 @@ gnutls_log_function2 (int level, const char *string, const char *extra)
   message ("gnutls.c: [%d] %s %s", level, string, extra);
 }
 
+#ifndef WINDOWSNT
+static int
+emacs_gnutls_non_blocking_errno(gnutls_transport_ptr_t ptr)
+{
+  switch (errno)
+    {
+    case EINPROGRESS:
+    case ENOTCONN:
+      return EAGAIN;
+    default:
+      return errno;
+    }
+}
+#endif
+
 int
 gnutls_try_handshake (struct Lisp_Process *proc)
 {
@@ -460,6 +479,11 @@ emacs_gnutls_handshake (struct Lisp_Process *proc)
       gnutls_transport_set_ptr2 (state,
 				 (void *) (intptr_t) proc->infd,
 				 (void *) (intptr_t) proc->outfd);
+      if (proc->is_non_blocking_client)
+        /* for non blocking connection
+           treat EINPROGRESS and ENOTCONN as EAGAIN */
+        gnutls_transport_set_errno_function(state,
+                                            emacs_gnutls_non_blocking_errno);
 #endif
 
       proc->gnutls_initstage = GNUTLS_STAGE_TRANSPORT_POINTERS_SET;
@@ -1596,8 +1620,16 @@ one trustfile (usually a CA bundle).  */)
 
   /* Call gnutls_init here: */
 
-  GNUTLS_LOG (1, max_log_level, "gnutls_init");
-  ret = gnutls_init (&state, GNUTLS_CLIENT);
+  if (XPROCESS (proc)->is_non_blocking_client)
+    {
+      GNUTLS_LOG (1, max_log_level, "gnutls_init with nonblocking");
+      ret = gnutls_init(&state, GNUTLS_CLIENT|GNUTLS_NONBLOCK);
+    }
+  else
+    {
+      GNUTLS_LOG (1, max_log_level, "gnutls_init");
+      ret = gnutls_init (&state, GNUTLS_CLIENT);
+    }
   XPROCESS (proc)->gnutls_state = state;
   if (ret < GNUTLS_E_SUCCESS)
     return gnutls_make_error (ret);
-- 
2.8.0