AnyEvent and Strawberry perl issue

[Steve Hay]

On 15 March 2015 at 05:26, Steve Hay <steve.m.hay at googlemail.com> wrote:
> On 14 March 2015 at 23:37, Marc Lehmann <schmorp at schmorp.de> wrote:
>> On Sat, Mar 14, 2015 at 02:49:34PM +0000, Steve Hay <steve.m.hay at googlemail.com> wrote:
>
>>> >             (connect $state{fh}, $sockaddr)
>>> >             || ($! == Errno::EINPROGRESS # POSIX
>>> >                 || $! == Errno::EWOULDBLOCK
>>> >                 # WSAEINPROGRESS intentionally not checked - it means something else entirely
>>> >                 || $! == AnyEvent::Util::WSAEINVAL # not convinced, but doesn't hurt
>>> >                 || $! == AnyEvent::Util::WSAEWOULDBLOCK)
>>> >
>>> > cygwin gets this right as well.
>>> >
>>> > So this change probably breaks connections because the WSA error code is
>>> > mapped on the wrong POSIX code. I don't think this can be fixed nicely
>>> > either, it needs to be fixed in perl, where the incorrect mapping is.
>>>
>>> I do not see how this will break.
>>
>> Of course not, that would have required some research and diligence. Or
>> maybe skimming my original mail, which explains it. Too difficult for you?
>>
>> I keep repeating myself, but here, again, in slow motion, just for you
>> who has so much trouble reading (or, as I have to suspect, ignores valid
>> arguments on purpose):
>>
>> 1. WSAEINPROGRESS has nothing whatsoever to do with EINPROGRESS. perl
>>    merged both to mean the same.
>> 2. if connect returns with an error of WSAEINPROGRESS, it means the connect
>>    has failed.
>> 3. if connect returns with an error of EINPROGRESS on a unix (or POSIX
>>    platform), it means the connect is under way, and the user will
>>    be notified.
>> 4. if connect returns with an error of EINPROGRESS on some perl win32
>>    configs, it means the connect has failed, and no notification will
>>    ever arrive.
>>
>> Code that relies on the correct error codes will timeout (at best) or
>> hang due to this change, instead of returning an error (because it will
>> misinterpret WSAEINPROGRESS as EINPROGRESS, which has completely different
>> meaning).
>>
>> AFAICS, this is true for every single case of WSAEINPROGRESS - it always
>> means that the operation has not been done, while the POSIX EINPROGRESS
>> code means the opposite.
>>
>>> So in all cases the test against EWOULDBLOCK works and there is no
>>> breakage.
>>
>> Obviously wrong, as was clear from the beginning. Do you see it now and
>> are you honest enough to admit to have fucked up?
>>
>
> Thank you for explaining this again. I'm sorry if you feel that my
> previous reply was intended as a smokescreen of any kind. I can only
> assure you that it wasn't meant that way. I was simply responding to
> points that you raised as I understood them. I do not think it would
> be productive to reply to every point one by one again now, so I will
> just reply to the main issue that I overlooked before:
>
> I did gloss over the (WSA)EINPROGRESS issue, wrongly believing that
> the check against EWOULDBLOCK would save the day in this case given
> that WSAEWOULDBLOCK is now mapped to EWOULDBLOCK and you had noted
> that "AnyEvent::Socket relies on the correct error code
> (WSAEWOULDBLOCK instead of EINPROGRESS)".
>
> Looking again, I can see how the code above is a problem when
> connect() fails on Windows and WSAGetLastError() returns
> WSAEINPROGRESS (10036). That return code is mapped to EINPROGRESS
> (112) before assignment to $!, and Errno::EINPROGRESS is also 112, so
> the test for $! == Errno::EINPROGRESS (which on *nix means the connect
> is under way) succeeds and the Windows code wrongly enters the
> "success" code when in fact connect() has failed.
>
> However, it is not the recent changes in question (i.e. those merged
> in by commit ea95436966) that have brought about this particular
> failure mode. Prior to those changes (i.e. in 5.19.3 and earlier) it
> is true that the WSAGetLastError() code (10036) would have been
> assigned directly to $! but the value of Errno::EINPROGRESS in
> pre-5.19.3 perls was also 10036 so the (*nix-oriented) test for
> whether $! == Errno::EINPROGRESS would still have wrongly succeeded
> then anyway, and the Windows code would still have wrongly entered the
> "success" code when connect() has failed.
>
> This has been the case for a very long time now, e.g. I just checked
> on 5.8.1 and Errno::EINPROGRESS is also 10036 there, matching the
> value assigned to $! on such a connect() failure.
>
> I am not saying that the WSAExxx / Exxx mapping is correct (indeed, it
> looks positively incorrect in the light of many examples that you've
> now given), but it had already been in place in some form for many,
> many years with no problems reported that I was aware of, so I did not
> think to question it when I added more of the same recently.
>
> I agree that it would be good to fix this if possible, although given
> that the origins of the WSAExxx / Exxx mapping are so old I would not
> see this as a showstopper for 5.22 (due in May).

I would like to add one more thing here. I realize that this has no
bearing on whether perl is correct in what it does (I'm agreeing that
it isn't, and hasn't been for a long time; it's a shame and a mystery
that it's taken over 13 years for the initial mistake to come to
light), but it's interesting to note that perl is far from alone in
having made this mistake: I've just downloaded the latest "stable"
source tarballs for four other popular scripting languages (Python,
Ruby, Tcl and PHP) to see what they do, and they all make the same
mistake as perl in mapping whole lists of Exxx / WSAExxx constants
with similar names, including EWOULDBLOCK and EINPROGRESS:

Python has this in Objects/exceptions.c:

#ifdef MS_WINDOWS
#include <winsock2.h>
/* The following constants were added to errno.h in VS2010 but have
   preferred WSA equivalents. */
[...]
#undef EINPROGRESS
[...]
#undef EWOULDBLOCK

[...]
#if defined(WSAEINPROGRESS) && !defined(EINPROGRESS)
#define EINPROGRESS WSAEINPROGRESS
#endif
[...]
#if defined(WSAEWOULDBLOCK) && !defined(EWOULDBLOCK)
#define EWOULDBLOCK WSAEWOULDBLOCK
#endif
#endif /* MS_WINDOWS */

Ruby has this in include/ruby/win32.h:

/* winsock error map */
#include <errno.h>

#ifndef EWOULDBLOCK
# define EWOULDBLOCK        WSAEWOULDBLOCK
#endif
#ifndef EINPROGRESS
# define EINPROGRESS        WSAEINPROGRESS
#endif
[...]

Tcl has this in win/tclWinError.c:

/*
 * The following table contains the mapping from WinSock errors to
 * errno errors.
 */

static const unsigned char wsaErrorTable[] = {
    EWOULDBLOCK,    /* WSAEWOULDBLOCK */
    EINPROGRESS,    /* WSAEINPROGRESS */
[...]

and PHP has this in ext/sockets/windows_common.h:

#ifdef ENETUNREACH /* errno.h probably included */
# undef EWOULDBLOCK
# undef EINPROGRESS
[...]
#endif

/* section disabled in WinSock2.h */
#define EWOULDBLOCK             WSAEWOULDBLOCK
#define EINPROGRESS             WSAEINPROGRESS
[...]