Hey friends,

Don't drink while reading this ;-) you'll get enough buzz on plain water ;-)

Intro:
------

Delivery of signals to a process/thread is actually
implementation/system dependent. The origin of the signal, and how it
is propagated to libcurl's process/thread is something completely out
of libcurl control.

System calls may be interrupted when a signal is received. In the
specific case of select/poll this translates into a select/poll result
of (-1) and errno being set to EINTR. When this happens select/poll
will return with no information relative to file descriptors being
monitorized and with specified timeout being ignored.

Given that libcurl must be capable of working on a large number of
very different systems, libcurl must be capable of smooth operation
even when select/poll is signal interrupted. This is achieved with a
combination of signal ignoring and short internal timeout values when
calling specific functions.

(1) select/poll loop:
---------------------

To allow signal ignoring a select/poll loop may be coded as follows:

| do {
| [...]
| r = poll(pfd, num, inner_timeout);
| if(r != -1)
| break;
| error = SOCKERRNO;
| if(error != EINTR)
| break;
| if(outer_timeout has expired)
| break;
| } while(r == -1);

This will loop if (select/poll has failed) and (errno is EINTR) and
(outer_timeout has not expired). IOW it loops at most outer_timeout
when select/poll is signal interrupted. This conforms to intended
default behavior.

If select/poll fails and does not set a non-zero errno, which violates
standard select/poll contract, then looping will not take place
subverting intended behavior.

(2) select/poll loop:
---------------------

Some might demand that signals are acknowledged and select/poll loop
may be coded as follows:

| do {
| [...]
| r = poll(pfd, num, inner_timeout);
| if(r != -1)
| break;
| error = SOCKERRNO;
| if(1)
| break;
| if(outer_timeout has expired)
| break;
| } while(r == -1);

This will not loop. This conforms to intended opt in behavior. This
will throw compiler warnings 'conditional expression is constant' on
line 'if(TRUE)' and 'unreachable code' on line if(outer_timeout has
expired)' and following ones.

If select/poll fails and does not set a non-zero errno, which violates
standard select/poll contract, just by chance there is no modification
of intended behavior given that errno is not checked.

(3) CURL_ACKNOWLEDGE_EINTR influence:
-------------------------------------

We can use CURL_ACKNOWLEDGE_EINTR being defined or not as a compile
time switch that makes select/poll loop get compiled as point (1) when
CURL_ACKNOWLEDGE_EINTR is not defined or compiled as point (2) when
CURL_ACKNOWLEDGE_EINTR is defined..

#ifdef CURL_ACKNOWLEDGE_EINTR
# define error_not_EINTR (1)
#else
# define error_not_EINTR (error != EINTR)
#endif

(4) select/poll loop:
---------------------

| do {
| [...]
| r = poll(pfd, num, inner_timeout);
| if(r != -1)
| break;
| error = SOCKERRNO;
| if(error_not_EINTR)
| break;
| if(outer_timeout has expired)
| break;
| } while(r == -1);

This will behave, when compiled, as select/poll loop (2) or (1) above
depending if CURL_ACKNOWLEDGE_EINTR is defined or not.

(5) extra errno check:
----------------------

We get rid of the compiler warnings without much fuss converting
'if(error_not_EINTR)' into 'if(error && error_not_EINTR)'.

This is what we actually have since 2007. So lets see which is the
influence of this extra errno checking.

loop (1) gets transformed into:

| do {
| [...]
| r = poll(pfd, num, inner_timeout);
| if(r != -1)
| break;
| error = SOCKERRNO;
| if(error && (error != EINTR))
| break;
| if(outer_timeout has expired)
| break;
| } while(r == -1);

This introduces a more restrictive condition relative to (1) to allow
looping when errno is EINTR, it introduces that errno must also be
different than zero. If select/poll is properly implemented this
represents no problem.

If select/poll fails without setting errno to non-zero, which violates
standard select/poll contract, this screws intended behavior, upon
select/poll failures loop will not exit unti outer_timeout has
expired.

loop (2) gets transformed into:

| do {
| [...]
| r = poll(pfd, num, inner_timeout);
| if(r != -1)
| break;
| error = SOCKERRNO;
| if(error && (1))
| break;
| if(outer_timeout has expired)
| break;
| } while(r == -1);

This introduces a more restrictive condition relative to (2) it
requires that errno is non-zero in order to avoid looping. If
select/poll is properly implemented this represents no problem.

If select/poll fails without setting errno to non-zero, which violates
standard select/poll contract, once more intended behavior is
subverted. Something that should not loop when select/poll fails gets
transformed into something that loops until outer_timeout has expired

(6) Considerations:
-------------------

It is clear that a select/poll failure that does not set errno to
non-zero subverts intended behavior of all these cases, except for one
that just by chance errno value is not checked.

The reasons behind select/poll failing and not setting errno to
non-zero for libcurl eyes are of two types.

- Non standard conforming system select/poll function which actually
does not set errno. This system should simply be considered as not
supported for proper libcurl operation, the same as many other
software.

- Bad or misconfigured build environment. System select/poll fails and
an errno variable is set to non-zero somewhere. When libcurl is built
it is instructed to check errno value, _BUT_ due to bad or
misconfigured build environment libcurl is fooled into checking an
errno value at a location that does not actually match the one set by
the system call.

(7) FAQ:
--------

- Should we 'fix' those loops in select.c?

It depends. Do we require a standard conformant select/poll?

If we require it, then I believe there's no actual need to change select.c

- Do the select/poll loops in select.c cause brain damage when reading
that code?.

No harm has been proved, even though it may cause nausea or dizziness
after a while ;-)

- Could we simplify that code somehow, while keeping existing
functionality, to cause less nausea?

Hmmm... Probably a static select/poll function wrapper with two
different implementations depending if CURL_ACKNOWLEDGE_EINTR is
defined or not could do the trick.

2011-08-18

-- 
-=[Yang]=-
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html