gnutls infloop possibly fixed

gnutls infloop possibly fixed

[Lars Ingebrigtsen]

I finally took a hard look at the GnuTLS-related Emacs hangs.  It turned
out to be pretty easy to reproduce for me.  If I switch off the wifi on
this laptop and then presses `g' in Gnus, Emacs would reliably use 100%
CPU, and `C-g' or anything wouldn't work.  So a totally dead Emacs.

I chased this down to emacs_gnutls_write, which would just, well,
infloop if libgnutls returned EAGAIN.  I've now removed that, and just
leaves the normal Emacs process loop to retry if we get an EAGAIN.

This seems to work for me -- `C-g' reliably works for me if I drop the
network, or I get a new IP address.

There may be other side-effects, though.  Slower throughput?  I don't
know.  Please give it a whirl and report back whether things stop
working or not.

-- 
(domestic pets only, the antidote for overdose, milk.)
  http://lars.ingebrigtsen.no  *  Sent from my Rome

Re: gnutls infloop possibly fixed

[Ted Zlatanov]

On Sat, 11 Feb 2012 18:10:23 +0100 Lars Ingebrigtsen <larsi@gnus.org> wrote: 

LI> I finally took a hard look at the GnuTLS-related Emacs hangs.  It turned
LI> out to be pretty easy to reproduce for me.  If I switch off the wifi on
LI> this laptop and then presses `g' in Gnus, Emacs would reliably use 100%
LI> CPU, and `C-g' or anything wouldn't work.  So a totally dead Emacs.

LI> I chased this down to emacs_gnutls_write, which would just, well,
LI> infloop if libgnutls returned EAGAIN.  I've now removed that, and just
LI> leaves the normal Emacs process loop to retry if we get an EAGAIN.

LI> This seems to work for me -- `C-g' reliably works for me if I drop the
LI> network, or I get a new IP address.

LI> There may be other side-effects, though.  Slower throughput?  I don't
LI> know.  Please give it a whirl and report back whether things stop
LI> working or not.

Thanks!  I have just one question: can we, instead of aborting on
EAGAIN, retry a few times?  My suggestion is below.  Maybe we could wait
a millisecond, too.  Or is all that handled at the Emacs process loop
level?

Thanks
Ted

=== modified file 'src/gnutls.c'
--- src/gnutls.c        2012-02-11 17:06:14 +0000
+++ src/gnutls.c        2012-02-12 02:46:24 +0000
@@ -341,6 +341,7 @@
 EMACS_INT
 emacs_gnutls_write (struct Lisp_Process *proc, const char *buf, EMACS_INT nbyte)
 {
+  int retries = 10;
   ssize_t rtnval = 0;
   EMACS_INT bytes_written;
   gnutls_session_t state = proc->gnutls_state;
@@ -365,6 +366,8 @@
        {
          if (rtnval == GNUTLS_E_INTERRUPTED)
            continue;
+          if (rtnval == GNUTLS_E_AGAIN && retries-- > 0)
+           continue;
          else
            break;
        }

Re: gnutls infloop possibly fixed

[Lars Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> Thanks!  I have just one question: can we, instead of aborting on
> EAGAIN, retry a few times?  My suggestion is below.

That would make sense, I guess.  I have no idea whether EAGAIN is a
common situation to be in when doing writes on a functioning network,
though.

That is, if EAGAIN "never" happens when the network is up, then adding
the loop doesn't help any.  But it doesn't hurt, either, so...

> Maybe we could wait a millisecond, too.  Or is all that handled at the
> Emacs process loop level?

The Emacs process loop will eventually try to write the stuff out again,
but how long it takes can be pretty random.  If Emacs is idle, it
retries about once a second, I think?  But if we've just said
`process-send-string' and then is in an `accept-process-output' loop
(which would be the common case), then it'll probably be retried sooner?
Perhaps?

Hey, I can just test that now by taking down the network and hitting
`g'...  let's see...

Ok:

[pid  5621] 20:45:22 sendto(13, "p`q\214\\\37\354\20QD\215\272$\t\241\347\273\343\316\322\247", 21, 0, NULL, 0) = -1 EAGAIN (Resource temporarily unavailable)
[pid  5621] 20:45:22 sendto(13, "p`q\214\\\37\354\20QD\215\272$\t\241\347\273\343\316\322\247", 21, 0, NULL, 0) = -1 EAGAIN (Resource temporarily unavailable)
[pid  5621] 20:45:22 sendto(13, "p`q\214\\\37\354\20QD\215\272$\t\241\347\273\343\316\322\247", 21, 0, NULL, 0) = -1 EAGAIN (Resource temporarily unavailable)

(etc)

It seems to retry the GnuTLS sendto about 20 times a second in the `g'
test.  I think that's probably more than good enough, and adding an
extra loop there isn't necessary.

-- 
(domestic pets only, the antidote for overdose, milk.)
  http://lars.ingebrigtsen.no  *  Sent from my Rome

Re: gnutls infloop possibly fixed

[Ted Zlatanov]

On Sun, 12 Feb 2012 20:47:14 +0100 Lars Ingebrigtsen <larsi@gnus.org> wrote: 

LI> Ted Zlatanov <tzz@lifelogs.com> writes:
>> Thanks!  I have just one question: can we, instead of aborting on
>> EAGAIN, retry a few times?  My suggestion is below.

...

LI> It seems to retry the GnuTLS sendto about 20 times a second in the `g'
LI> test.  I think that's probably more than good enough, and adding an
LI> extra loop there isn't necessary.

Cool, thanks for testing that.  I really appreciate your fix.

Ted

Re: gnutls infloop possibly fixed

[Tom Tromey]

>>>>> "Lars" == Lars Ingebrigtsen <larsi@gnus.org> writes:

Lars> That would make sense, I guess.  I have no idea whether EAGAIN is a
Lars> common situation to be in when doing writes on a functioning network,
Lars> though.

I use one nntp server that causes Emacs to pause for quite a while as it
works something out during the connection.  If I strace Emacs, I see it
looping on select -> read -> EAGAIN.  Then eventually something happens
and things return back to normal.

I'm wondering whether this is a bug in GNUTLS, or Emacs' use thereof, or
the server.  Any ideas offhand?

Tom

Re: gnutls infloop possibly fixed

[Lars Ingebrigtsen]

Tom Tromey <tromey@redhat.com> writes:

> I use one nntp server that causes Emacs to pause for quite a while as it
> works something out during the connection.  If I strace Emacs, I see it
> looping on select -> read -> EAGAIN.  Then eventually something happens
> and things return back to normal.

A read -> EAGAIN normally means that the server didn't return anything
(much).  So it kinda sounds like a slow NNTP server.

-- 
(domestic pets only, the antidote for overdose, milk.)
  http://lars.ingebrigtsen.no  *  Sent from my Rome

Re: gnutls infloop possibly fixed

[Lars Ingebrigtsen]

I had a bug report of a strange file-error "success" being reported by a
Gnus user, and I applied the following patch.  It sets errno to EAGAIN
if GnuTLS returns EAGAIN.

Does this make sense, or is it just cargo-cult programming?

=== modified file 'src/gnutls.c'
*** src/gnutls.c	2012-02-11 17:06:14 +0000
--- src/gnutls.c	2012-02-13 19:35:30 +0000
***************
*** 365,372 ****
  	{
  	  if (rtnval == GNUTLS_E_INTERRUPTED)
  	    continue;
! 	  else
  	    break;
  	}
  
        buf += rtnval;
--- 365,384 ----
  	{
  	  if (rtnval == GNUTLS_E_INTERRUPTED)
  	    continue;
! 	  else {
! 	    /* If we get EAGAIN, then set errno appropriately so that
! 	       send_process retries the correct way instead of
! 	       erroring out. */
! 	    if (rtnval == EAGAIN) {
! #ifdef EWOULDBLOCK
! 	      errno = EWOULDBLOCK;
! #endif
! #ifdef EAGAIN
! 	      errno = EAGAIN;
! #endif
! 	    }
  	    break;
+ 	  }
  	}
  
        buf += rtnval;


-- 
(domestic pets only, the antidote for overdose, milk.)
  http://lars.ingebrigtsen.no  *  Sent from my Rome

Re: gnutls infloop possibly fixed

[Andreas Schwab]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> ! 	    if (rtnval == EAGAIN) {

That is never true.

> ! #ifdef EWOULDBLOCK
> ! 	      errno = EWOULDBLOCK;
> ! #endif
> ! #ifdef EAGAIN
> ! 	      errno = EAGAIN;
> ! #endif

Does this actually survive emacs_gnutls_handle_error?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: gnutls infloop possibly fixed

[Lars Ingebrigtsen]

Andreas Schwab <schwab@linux-m68k.org> writes:

>> ! 	    if (rtnval == EAGAIN) {
>
> That is never true.

D'oh.  It's GNUTLS_E_EAGAIN, I guess.

-- 
(domestic pets only, the antidote for overdose, milk.)
  http://lars.ingebrigtsen.no  *  Sent from my Rome

Re: gnutls infloop possibly fixed

[Richard Stallman]

    > Thanks!  I have just one question: can we, instead of aborting on
    > EAGAIN, retry a few times?  My suggestion is below.

    That would make sense, I guess.  I have no idea whether EAGAIN is a
    common situation to be in when doing writes on a functioning network,
    though.

EAGAIN is defined to mean that a system call was interrupted and the
program should try again.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use free telephony http://directory.fsf.org/category/tel/

Re: gnutls infloop possibly fixed

[Andreas Schwab]

Richard Stallman <rms@gnu.org> writes:

> EAGAIN is defined to mean that a system call was interrupted and the
> program should try again.

That's EINTR.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."