esp32/modsocket: Handle poll of a closed socket.

[dpgeorge]

This gets tests/extmod/uselect_poll_basic.py working on the esp32.

@tve FYI, this should fix the issue you found with uasyncio and write after closing the connection.

[dpgeorge]

Merged in bf4fb16

[tve]

This PR has changed the error code returned when connecting to a port that has no listener from the correct 111/ECONNREFUSED to an incorrect 104/ERESET. I.e., connect to some random port on some server and now you get a 104, previously you got the correct 111.

[dpgeorge]

This PR has changed the error code returned when connecting to a port that has no listener from the correct 111/ECONNREFUSED to an incorrect 104/ERESET.

I'm not sure it was this commit that did that. Going back to the commit just before this one it looks like the same behaviour is there, that it raises OSError(104) for a refused connection due to a bad port.

The test I used was:

# test that socket.connect() has correct behaviour connecting to a port that has no listener

try:
    import usocket as socket, uselect as select, uerrno as errno
except:
    import socket, select, errno

PORT_NO_LISTNER = 81


def test(peer_addr):
    s = socket.socket()

    try:
        s.connect(peer_addr)
    except OSError as er:
        print("OSError ECONNREFUSED:", er.args[0] - errno.ECONNREFUSED)


if __name__ == "__main__":
    test(socket.getaddrinfo('micropython.org', PORT_NO_LISTNER)[0][-1])

This works correctly on CPython and micropython unix.

It doesn't work on stm32 or esp32 (an probably also esp8266), instead it returns 104 (not 111). It looks like this is due to how lwIP handles the refused connection.

[tve]

Yup, my bad. I know my test case passed but I obviously got mixed-up about where it passed :-(. My apologies for the wild goose chase. I did work on your test case a little more to test non-blocking sockets, since that's what uasyncio streams use:

# test that socket.connect() has correct behaviour connecting to a port that has no listener

try:
    import usocket as socket, uselect as select, uerrno as errno
except:
    import socket, select, errno

PORT_NO_LISTNER = 81


def test(peer_addr):
    # test blocking socket
    s = socket.socket()
    try:
        s.connect(peer_addr)
    except OSError as e:
        print("Blocking connect says:", e)

    # test non-blocking socket
    s = socket.socket()
    s.setblocking(False)

    try:
        s.connect(peer_addr)
    except OSError as e:
        print("Non-blocking connect says:", e)

    try:
        while True:
            try:
                s.send(b'hello')
                break
            except OSError as e:
                if e.args[0] != 11 and e.args[0] != 115: # 11=EGAIN, 115=EINPROGRESS
                    raise
    except OSError as e:
        print("send says:", e)


if __name__ == "__main__":
    test(socket.getaddrinfo('micropython.org', PORT_NO_LISTNER)[0][-1])

Sample runs on CPython, linux MP, and esp32:

> python test-openfail.py
Blocking connect says: [Errno 111] Connection refused
Non-blocking connect says: [Errno 115] Operation now in progress
send says: [Errno 111] Connection refused
> ../../micropython/ports/unix/micropython test-openfail.py
Blocking connect says: [Errno 111] ECONNREFUSED
Non-blocking connect says: [Errno 115] EINPROGRESS
send says: [Errno 111] ECONNREFUSED
> ./pyb test-openfail.py
Blocking connect says: [Errno 104] ECONNRESET
Non-blocking connect says: [Errno 115] EINPROGRESS
send says: [Errno 104] ECONNRESET

I realize you need a slightly different output format for the regression tests.

[dpgeorge]

Do you consider this something which should be fixed? It may not be possible to fix if lwip cannot distinguish between errno 104 and 111, but an option would be to convert the errno's for the situations where we know they should be converted.

[tve]

Thanks for asking.
Looking at the table that maps LwIP errors to posix errors:

micropython/ports/esp32/lwip_err.c

Lines 43 to 63 in 577659a

	/** Table to quickly map an lwIP error (err_t) to a socket error
	* by using -err as an index */
	static const int err_to_errno_table[] = {
	0, /* ERR_OK 0 No error, everything OK. */
	MP_ENOMEM, /* ERR_MEM -1 Out of memory error. */
	MP_ENOBUFS, /* ERR_BUF -2 Buffer error. */
	MP_EWOULDBLOCK, /* ERR_TIMEOUT -3 Timeout */
	MP_EHOSTUNREACH, /* ERR_RTE -4 Routing problem. */
	MP_EINPROGRESS, /* ERR_INPROGRESS -5 Operation in progress */
	MP_EINVAL, /* ERR_VAL -6 Illegal value. */
	MP_EWOULDBLOCK, /* ERR_WOULDBLOCK -7 Operation would block. */
	MP_EADDRINUSE, /* ERR_USE -8 Address in use. */
	MP_EALREADY, /* ERR_ALREADY -9 Already connecting. */
	MP_EISCONN, /* ERR_ISCONN -10 Conn already established.*/
	MP_ENOTCONN, /* ERR_CONN -11 Not connected. */
	-1, /* ERR_IF -12 Low-level netif error */
	MP_ECONNABORTED, /* ERR_ABRT -13 Connection aborted. */
	MP_ECONNRESET, /* ERR_RST -14 Connection reset. */
	MP_ENOTCONN, /* ERR_CLSD -15 Connection closed. */
	MP_EIO /* ERR_ARG -16 Illegal argument. */
	};

I don't see a "connection refused". For blocking sockets we could map reset->refused. For non-blocking we could keep track of bytes transferred and if zero do that mapping as well.
Overall, I would say "got bigger fish to fry" and leave it as-is. I'll add a little comment to socket_connect for the future...

[dpgeorge]

What is the intention of the send test in the above script? On PYBD (using modlwip.c) the send actually goes through... (probably that's a bug but not sure if its' in lwip or elsewhere).

[tve]

I've evolved this test a lot further in one of the later PRs. The intention was to test that a send after a non-blocking connect return an appropriate error code. The new test is in #5825: https://github.com/micropython/micropython/blob/3c67ef8ea9a9e934be4e690d70d243688ebaa6db/tests/net_hosted/connect_nonblock.py
It uses a non-existent IP address to ensure that the connect doesn't suceed before send/write/recv/read is issued and its error code is validated. The SSL part of the test fails with AXTLS because while one can delay the handshake at connect it blocks on it at the first read or write.

[tve]

I should note that I've resolved most of the error code issues in the PRs queued, at least on the platforms I've been able to test. It's not all 100% to standard but where it's not I don't think it's reasonably possible.

[dpgeorge]

I should note that I've resolved most of the error code issues in the PRs queued, at least on the platforms I've been able to test. It's not all 100% to standard but where it's not I don't think it's reasonably possible.

Ok, then let's leave this issue here as-is, to be resolved later in those PRs.