Calling 'select' from emacs_gnutls_pull

Calling 'select' from emacs_gnutls_pull

[Eli Zaretskii]

Ted,

I need your help in understanding the implementation of
emacs_gnutls_pull.  Specifically, why does it need to call 'select'
when 'sys_read' returns EWOULDBLOCK?  The file descriptor should
already be watched by the call to 'select' in
'wait_reading_process_output', so it seems like a call to 'sys_read'
is all that's needed here.  Am I missing something?

The reason I ask is that someone reported seeing these messages when
running Emacs on Windows under a debugger:

  warning: sys_read called when read is in progress

This comes from 'sys_read', and it means that the reader thread (used
by Emacs to read from a socket) is already blocked inside a call to
'recv' on behalf of this file descriptor, when 'sys_read' is called.

I think that the sequence of events which leads to this is as follows:

  . emacs_gnutls_pull calls sys_read, which returns EWOULDBLOCK,
    because there's no data ready to be read

  . emacs_gnutls_pull then calls select, which wakes up the reader
    thread and tells it to try reading from the socket

  . the reader thread blocks inside the call to recv (since no data is
    ready)

  . the call to select times out and returns EWOULDBLOCK, which causes
    emacs_gnutls_pull to break from the loop and return EAGAIN

  . after some time, emacs_gnutls_pull is called again, and calls
    sys_read while the reader thread is still blocked in recv, so we
    get the above message

Is this a plausible description of what might happen?  If so, would it
make sense to avoid calling 'select' in emacs_gnutls_pull, and instead
let 'wait_reading_process_output' call 'select', as it does for all
the other sockets open in Emacs?  Or am I missing something?

TIA

Re: Calling 'select' from emacs_gnutls_pull

[Ted Zlatanov]

On Sat, 16 Feb 2013 13:38:43 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 

EZ> I need your help in understanding the implementation of
EZ> emacs_gnutls_pull.  Specifically, why does it need to call 'select'
EZ> when 'sys_read' returns EWOULDBLOCK?  The file descriptor should
EZ> already be watched by the call to 'select' in
EZ> 'wait_reading_process_output', so it seems like a call to 'sys_read'
EZ> is all that's needed here.  Am I missing something?

Well, I *think* the function `emacs_gnutls_pull' is called in a
different context from most of Emacs.  In gnutls.c we set that function
as the transport pull at the GnuTLS level:

fn_gnutls_transport_set_push_function (state, &emacs_gnutls_push);
fn_gnutls_transport_set_pull_function (state, &emacs_gnutls_pull);

...and there are comments accumulated over the years in that
neighborhood.  Claudio Bley contributed much of the original code here,
including the piece you're referring to.  The history is at

http://comments.gmane.org/gmane.emacs.devel/136816

EZ> The reason I ask is that someone reported seeing these messages when
EZ> running Emacs on Windows under a debugger:

EZ>   warning: sys_read called when read is in progress

EZ> This comes from 'sys_read', and it means that the reader thread (used
EZ> by Emacs to read from a socket) is already blocked inside a call to
EZ> 'recv' on behalf of this file descriptor, when 'sys_read' is called.

OK.

EZ> I think that the sequence of events which leads to this is as follows:

EZ>   . emacs_gnutls_pull calls sys_read, which returns EWOULDBLOCK,
EZ>     because there's no data ready to be read

EZ>   . emacs_gnutls_pull then calls select, which wakes up the reader
EZ>     thread and tells it to try reading from the socket

EZ>   . the reader thread blocks inside the call to recv (since no data is
EZ>     ready)

EZ>   . the call to select times out and returns EWOULDBLOCK, which causes
EZ>     emacs_gnutls_pull to break from the loop and return EAGAIN

EZ>   . after some time, emacs_gnutls_pull is called again, and calls
EZ>     sys_read while the reader thread is still blocked in recv, so we
EZ>     get the above message

EZ> Is this a plausible description of what might happen?  If so, would it
EZ> make sense to avoid calling 'select' in emacs_gnutls_pull, and instead
EZ> let 'wait_reading_process_output' call 'select', as it does for all
EZ> the other sockets open in Emacs?  Or am I missing something?

I think you're correct, but remember the GnuTLS library is calling these
functions.  So I don't know if it would make sense to let the Emacs
reader thread handle the socket management, because we could end up with
poor performance or even contention.  I just don't know the intricacies
of the W32 I/O system and how it interacts with GnuTLS well enough.

The best way is to ask the GnuTLS developers and to test it, I think.
There are some hints at

http://www.gnu.org/software/gnutls/manual/html_node/Asynchronous-operation.html
http://stackoverflow.com/questions/911565/select-recv-and-ewouldblock-on-non-blocking-sockets

Sorry this is all I can suggest right now, but if I can help simulate
this condition or test some possible solutions, I'll be glad to assist.

Ted

Re: Calling 'select' from emacs_gnutls_pull

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Date: Sat, 16 Feb 2013 10:55:45 -0500
> 
> ...and there are comments accumulated over the years in that
> neighborhood.  Claudio Bley contributed much of the original code here,
> including the piece you're referring to.  The history is at
> 
> http://comments.gmane.org/gmane.emacs.devel/136816

I've (re-)read that thread, but didn't find any rationale for calling
'select'.  Claudio, could you perhaps chime in?

> I think you're correct, but remember the GnuTLS library is calling these
> functions.

But GnuTLS calls these functions because Emacs called
'emacs_gnutls_read', right?  Which seems to be just a fancy way of
reading from a socket, as far as Emacs is concerned, as this fragment
from 'read_process_output', a subroutine of 'wait_reading_process_output',
seems to indicate:

  #ifdef HAVE_GNUTLS
	if (p->gnutls_p)
	  nbytes = emacs_gnutls_read (p, chars + carryover + buffered,
				      readmax - buffered);
	else
  #endif
	  nbytes = emacs_read (channel, chars + carryover + buffered,
			       readmax - buffered);

Here, 'emacs_read' (on Windows) just calls 'sys_read'.  But
'emacs_gnutls_read', via 'emacs_gnutls_pull', in addition also calls
'select', and I wonder why this difference is necessary.

> So I don't know if it would make sense to let the Emacs
> reader thread handle the socket management, because we could end up with
> poor performance or even contention.

We already use the Emacs reader threads for that.  And it works well
enough for sockets.  In a nutshell, the reader thread does blocking
reads from the socket, while 'sys_read' (called by Emacs as 'read')
emulates non-blocking reads, by looking at the status set by the
reader thread.

> if I can help simulate this condition or test some possible
> solutions, I'll be glad to assist.

Thanks.  It would be good if you (or someone else) could describe a
simple setup for reading stuff via GnuTLS from some URL, which I could
then try on my machine under a debugger.

Re: Calling 'select' from emacs_gnutls_pull

[Ted Zlatanov]

On Sat, 16 Feb 2013 18:52:23 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 

>> From: Ted Zlatanov <tzz@lifelogs.com>
>> Date: Sat, 16 Feb 2013 10:55:45 -0500

>> So I don't know if it would make sense to let the Emacs
>> reader thread handle the socket management, because we could end up with
>> poor performance or even contention.

EZ> We already use the Emacs reader threads for that.  And it works well
EZ> enough for sockets.  In a nutshell, the reader thread does blocking
EZ> reads from the socket, while 'sys_read' (called by Emacs as 'read')
EZ> emulates non-blocking reads, by looking at the status set by the
EZ> reader thread.

Nice.  The simpler, the better!

>> if I can help simulate this condition or test some possible
>> solutions, I'll be glad to assist.

EZ> Thanks.  It would be good if you (or someone else) could describe a
EZ> simple setup for reading stuff via GnuTLS from some URL, which I could
EZ> then try on my machine under a debugger.

Sure.  It's actually quite simple to open a SSL connection to the HTTP/S
port (the SERVICE parameter is either a string or an integer):

#+begin_src lisp
(open-gnutls-stream "tls" "tls-buffer" "yourserver.com" "https")
#+end_src

If GnuTLS support is enabled, opening any SSL connection with the
`url-*' function should do it too... it's the default transport.

You can also call `gnutls-negotiate' directly, see `gnutls.el' for the
details.

Ted

Re: Calling 'select' from emacs_gnutls_pull

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Date: Sat, 16 Feb 2013 13:00:58 -0500
> 
> EZ> Thanks.  It would be good if you (or someone else) could describe a
> EZ> simple setup for reading stuff via GnuTLS from some URL, which I could
> EZ> then try on my machine under a debugger.
> 
> Sure.  It's actually quite simple to open a SSL connection to the HTTP/S
> port (the SERVICE parameter is either a string or an integer):
> 
> #+begin_src lisp
> (open-gnutls-stream "tls" "tls-buffer" "yourserver.com" "https")
> #+end_src
> 
> If GnuTLS support is enabled, opening any SSL connection with the
> `url-*' function should do it too... it's the default transport.
> 
> You can also call `gnutls-negotiate' directly, see `gnutls.el' for the
> details.

OK.  Any suggestions for yourserver.com that I could use?

Also, would there be any problems with certificates, and if so, where
can I find a bundle?  (I'm quite ignorant about these issues, sorry.)

Thanks.

Re: Calling 'select' from emacs_gnutls_pull

[Ted Zlatanov]

On Sat, 16 Feb 2013 21:30:53 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 

>> From: Ted Zlatanov <tzz@lifelogs.com>
>> Date: Sat, 16 Feb 2013 13:00:58 -0500
>> 
EZ> Thanks.  It would be good if you (or someone else) could describe a
EZ> simple setup for reading stuff via GnuTLS from some URL, which I could
EZ> then try on my machine under a debugger.
>> 
>> Sure.  It's actually quite simple to open a SSL connection to the HTTP/S
>> port (the SERVICE parameter is either a string or an integer):
>> 
>> #+begin_src lisp
>> (open-gnutls-stream "tls" "tls-buffer" "yourserver.com" "https")
>> #+end_src
>> 
>> If GnuTLS support is enabled, opening any SSL connection with the
>> `url-*' function should do it too... it's the default transport.
>> 
>> You can also call `gnutls-negotiate' directly, see `gnutls.el' for the
>> details.

EZ> OK.  Any suggestions for yourserver.com that I could use?

https://www.gnu.org or https://www.google.com

EZ> Also, would there be any problems with certificates, and if so, where
EZ> can I find a bundle?  (I'm quite ignorant about these issues, sorry.)

I believe by default, only warnings are printed about certificate
mismatch so you should be OK.  Look at `gnutls-trustfiles' if you want
to customize the bundles.

Ted

Re: Calling 'select' from emacs_gnutls_pull

[Claudio Bley]

Hi.

At Sat, 16 Feb 2013 18:52:23 +0200,
Eli Zaretskii wrote:
> 
> > From: Ted Zlatanov <tzz@lifelogs.com>
> > Date: Sat, 16 Feb 2013 10:55:45 -0500
> > 
> > ...and there are comments accumulated over the years in that
> > neighborhood.  Claudio Bley contributed much of the original code here,
> > including the piece you're referring to.  The history is at
> > 
> > http://comments.gmane.org/gmane.emacs.devel/136816
> 
> I've (re-)read that thread, but didn't find any rationale for calling
> 'select'.  Claudio, could you perhaps chime in?

That was quite some time ago and, I'm afraid, my memory fails me at
that.

Looking at the code, I cannot see a strong reason calling select
inside of emacs_gnutls_pull... Maybe it was indeed because of
performance reasons, but more likely some kind of left-over from
chasing a red herring while trying to figure out why it didn't work
sometimes.

However, applying the obvious patch:

--- >8 ---
--- src/w32.c	Thu Feb 21 14:35:43 2013
+++ ../build/src/w32.c	Fri Feb 22 08:32:35 2013
@@ -7837,27 +7837,9 @@
 
       err = errno;
 
+      /* Translate the WSAEWOULDBLOCK alias EWOULDBLOCK to EAGAIN. */
       if (err == EWOULDBLOCK)
-        {
-          /* Set a small timeout.  */
-	  timeout = make_emacs_time (1, 0);
-          FD_ZERO (&fdset);
-          FD_SET ((int)fd, &fdset);
-
-          /* Use select with the timeout to poll the selector.  */
-          sc = select (fd + 1, &fdset, (SELECT_TYPE *)0, (SELECT_TYPE *)0,
-                       &timeout, NULL);
-
-          if (sc > 0)
-            continue;  /* Try again.  */
-
-          /* Translate the WSAEWOULDBLOCK alias EWOULDBLOCK to EAGAIN.
-             Also accept select return 0 as an indicator to EAGAIN.  */
-          if (sc == 0 || errno == EWOULDBLOCK)
-            err = EAGAIN;
-          else
-            err = errno; /* Other errors are just passed on.  */
-        }
+        err = EAGAIN;
 
       emacs_gnutls_transport_set_errno (process->gnutls_state, err);
 

--- >8 ---

and made a few tests using POP3 and IMAP connections over TLS and
HTTPS connections. Works good.

I also prepared a small test driver, downloading a file 33 MiByte in
size from a remote host (over VPN):

,----[ test_tls.el ]
| (require 'gnutls)
| 
| (when (gnutls-available-p)
|   (url-retrieve-synchronously "https://192.168.0.1/artifactory-2.4.1.zip"))
| 
| (kill-emacs)
`----

Here are some numbers running emacs with "-Q --load test_tls.el" a few
times. Note, that using "--batch -Q --script test_tls.el" uses 100%
CPU and exits with an out of memory error (I assume because the GC
is never run since there is no event loop nor threads involved).

Average for with-select key over 3 runs

Version Number:   Windows NT 5.1 (Build 2600)
Exit Time:        1:00 am, Monday, January 1 1601
Elapsed Time:     0:00:35.208
Process Time:     0:00:05.822
System Calls:     659366     
Context Switches: 138074     
Page Faults:      61507      
Bytes Read:       80719977   
Bytes Written:    320584     
Bytes Other:      908395     


Average for without-select key over 3 runs

Version Number:   Windows NT 5.1 (Build 2600)
Exit Time:        1:00 am, Monday, January 1 1601
Elapsed Time:     0:00:33.796
Process Time:     0:00:06.166
System Calls:     857893     
Context Switches: 140154     
Page Faults:      69061      
Bytes Read:       99353998   
Bytes Written:    2017152    
Bytes Other:      1175082    


-- 
Claudio
-- 
Claudio

Re: Calling 'select' from emacs_gnutls_pull

[Eli Zaretskii]

> From: claudio.bley@gmail.com (Claudio Bley)
> Date: Fri, 22 Feb 2013 12:15:36 +0100
> 
> However, applying the obvious patch:
> 
> --- >8 ---
> --- src/w32.c	Thu Feb 21 14:35:43 2013
> +++ ../build/src/w32.c	Fri Feb 22 08:32:35 2013
> @@ -7837,27 +7837,9 @@
>  
>        err = errno;
>  
> +      /* Translate the WSAEWOULDBLOCK alias EWOULDBLOCK to EAGAIN. */
>        if (err == EWOULDBLOCK)
> -        {
> -          /* Set a small timeout.  */
> -	  timeout = make_emacs_time (1, 0);
> -          FD_ZERO (&fdset);
> -          FD_SET ((int)fd, &fdset);
> -
> -          /* Use select with the timeout to poll the selector.  */
> -          sc = select (fd + 1, &fdset, (SELECT_TYPE *)0, (SELECT_TYPE *)0,
> -                       &timeout, NULL);
> -
> -          if (sc > 0)
> -            continue;  /* Try again.  */
> -
> -          /* Translate the WSAEWOULDBLOCK alias EWOULDBLOCK to EAGAIN.
> -             Also accept select return 0 as an indicator to EAGAIN.  */
> -          if (sc == 0 || errno == EWOULDBLOCK)
> -            err = EAGAIN;
> -          else
> -            err = errno; /* Other errors are just passed on.  */
> -        }
> +        err = EAGAIN;
>  
>        emacs_gnutls_transport_set_errno (process->gnutls_state, err);
>  
> 
> --- >8 ---
> 
> and made a few tests using POP3 and IMAP connections over TLS and
> HTTPS connections. Works good.

That's good news.

> Average for with-select key over 3 runs
> 
> Version Number:   Windows NT 5.1 (Build 2600)
> Exit Time:        1:00 am, Monday, January 1 1601
> Elapsed Time:     0:00:35.208
> Process Time:     0:00:05.822
> System Calls:     659366     
> Context Switches: 138074     
> Page Faults:      61507      
> Bytes Read:       80719977   
> Bytes Written:    320584     
> Bytes Other:      908395     
> 
> 
> Average for without-select key over 3 runs
> 
> Version Number:   Windows NT 5.1 (Build 2600)
> Exit Time:        1:00 am, Monday, January 1 1601
> Elapsed Time:     0:00:33.796
> Process Time:     0:00:06.166
> System Calls:     857893     
> Context Switches: 140154     
> Page Faults:      69061      
> Bytes Read:       99353998   
> Bytes Written:    2017152    
> Bytes Other:      1175082    

The times look comparable, but I'm confused why byte counts are so
much different.  Is there a good explanation to that?

Anyway, AFAIU, your tests indicate that it would be okay to install
the above changes on the development trunk, right?

Thanks for your valuable feedback.

Re: Calling 'select' from emacs_gnutls_pull

[Claudio Bley]

At Fri, 22 Feb 2013 14:36:46 +0200,
Eli Zaretskii wrote:
> 
> > From: claudio.bley@gmail.com (Claudio Bley)
> > Date: Fri, 22 Feb 2013 12:15:36 +0100
> > 
> > Average for with-select key over 3 runs
> > 
> > Version Number:   Windows NT 5.1 (Build 2600)
> > Exit Time:        1:00 am, Monday, January 1 1601
> > Elapsed Time:     0:00:35.208
> > Process Time:     0:00:05.822
> > System Calls:     659366     
> > Context Switches: 138074     
> > Page Faults:      61507      
> > Bytes Read:       80719977   
> > Bytes Written:    320584     
> > Bytes Other:      908395     
> > 
> > 
> > Average for without-select key over 3 runs
> > 
> > Version Number:   Windows NT 5.1 (Build 2600)
> > Exit Time:        1:00 am, Monday, January 1 1601
> > Elapsed Time:     0:00:33.796
> > Process Time:     0:00:06.166
> > System Calls:     857893     
> > Context Switches: 140154     
> > Page Faults:      69061      
> > Bytes Read:       99353998   
> > Bytes Written:    2017152    
> > Bytes Other:      1175082    
> 
> The times look comparable, but I'm confused why byte counts are so
> much different.  Is there a good explanation to that?

Probably an effect of a cold cache or something. Hard to say, I
already rm'ed the timeit database.

So, I quickly re-ran the tests (with one warming round). Here are the
results:

Average for with-select key over 3 runs

Version Number:   Windows NT 5.1 (Build 2600)
Exit Time:        1:00 am, Monday, January 1 1601
Elapsed Time:     0:00:34.875
Process Time:     0:00:05.619
System Calls:     651840     
Context Switches: 154225     
Page Faults:      62383      
Bytes Read:       81394755   
Bytes Written:    340437     
Bytes Other:      1069175    


Average for without-select key over 3 runs

Version Number:   Windows NT 5.1 (Build 2600)
Exit Time:        1:00 am, Monday, January 1 1601
Elapsed Time:     0:00:35.515
Process Time:     0:00:05.687
System Calls:     865096     
Context Switches: 145642     
Page Faults:      69529      
Bytes Read:       96329391   
Bytes Written:    361675     
Bytes Other:      1083219    

> Anyway, AFAIU, your tests indicate that it would be okay to install
> the above changes on the development trunk, right?

Yes.

-- 
Claudio

Re: Calling 'select' from emacs_gnutls_pull

[Eli Zaretskii]

> From: claudio.bley@gmail.com (Claudio Bley)
> Date: Fri, 22 Feb 2013 16:17:27 +0100
> 
> > Anyway, AFAIU, your tests indicate that it would be okay to install
> > the above changes on the development trunk, right?
> 
> Yes.

Thanks, installed.  I also removed the loop, which is now always
iterated exactly once.