Socket closing if timed out and no data on socket

[brentru]

If no data is received on a socket, and the socket has timed out, a socket will close. Subsequent reads raise OSError: [Errno 9] EBADF.

I dug deeper, this behavior is expected (https://github.com/adafruit/circuitpython/blob/main/ports/esp32s2/common-hal/socketpool/Socket.c#L123) on the esp32-s2.

However, closing the socket immediately makes it difficult to poll a socket to check if data is available.

For example, polling a socket for a response, like in MiniMQTT (https://github.com/brentru/Adafruit_CircuitPython_MiniMQTT/blob/cpython-s2/adafruit_minimqtt/adafruit_minimqtt.py#L872), raises OSError instead of simply timing out and leaving the socket open.

0
bytearray(b'\n')
recv from socket...
Traceback (most recent call last):
  File "code.py", line 47, in <module>
OSError: [Errno 9] EBADF

Could we instead raise a timeout or perform some other behavior? CPython raises a socket.timeout (https://docs.python.org/3.8/library/socket.html#socket.timeout) error for this operation and leaves the socket open.

---

Code example, , receiving one byte of code at a time from a server, until it reads 0:

import time
import ssl
import socketpool
import wifi
import board
import neopixel

# Add a secrets.py to your filesystem that has a dictionary called secrets with "ssid" and
# "password" keys with your WiFi credentials. DO NOT share that file or commit it into Git or other
# source control.
# pylint: disable=no-name-in-module,wrong-import-order
try:
    from secrets import secrets
except ImportError:
    print("WiFi secrets are kept in secrets.py, please add them there!")
    raise

print("Connecting to %s"%secrets["ssid"])
wifi.radio.connect(secrets["ssid"], secrets["password"])
print("Connected to %s!"%secrets["ssid"])

# Create a socket pool
pool = socketpool.SocketPool(wifi.radio)

# Host
host = "0.tcp.ngrok.io"
port = 10480

addr_info = pool.getaddrinfo(
    host, port, 0, pool.SOCK_STREAM
)[0]

sock = pool.socket(
    addr_info[0], addr_info[1], addr_info[2]
)

sock.connect((host, port))
sock.settimeout(1)

buf = bytearray(1)
# attempt to recv 
while True:
    print("recv from socket...")
    time.sleep(2)
    data = sock.recv_into(buf, 1)
    print(data)
    print(buf)

Then I set up a simple tcp packet server using netcat (nc -l 23999)

Output:

Connected to!
recv from socket...
1
bytearray(b'h')
recv from socket...
1
bytearray(b'e')
recv from socket...
1
bytearray(b'l')
recv from socket...
1
bytearray(b'l')
recv from socket...
1
bytearray(b'o')
recv from socket...
1
bytearray(b'\n')
recv from socket...
0
bytearray(b'\n')
recv from socket...
Traceback (most recent call last):
  File "code.py", line 47, in <module>
OSError: [Errno 9] EBADF

[ladyada]

@tannewt hihi please take a look at this issue some time this week, @brentru is blocked on getting MQTT working on ESP32S2 because of it :)

[hierophect]

@ladyada I can fold this into #3854 or do a separate quickfix for it if you'd like. I'd considered removing the block of code that does this myself, but thought it might have been intentional behavior for one reason or another.

[ladyada]

@hierophect please chat with @tannewt and @brentru, im not the one using the code :)

[tannewt]

@hierophect @brentru You can't just remove the socket close because it is valid sometimes. I think what's missing is a test against the timeout condition. If the while loop has timed out, then we don't want to close the socket.

[tannewt]

I've followed up with recommendations on #3854

[hierophect]

@tannewt can you double check if you submitted your review? I don't see anything on my PR.

[tannewt]

@tannewt can you double check if you submitted your review? I don't see anything on my PR.

Yup! They were pending. Sorry about that. You should see them now. Thanks for the ping.

[hierophect]

@tannewt I wanted to follow up with you on this issue to talk more about the socket close behavior. Under what conditions do you want sockets to close automatically (ie, what's the scenario where they could leak when using recv, as you mentioned)? The Cpython docs don't say anything about automatic closes, and I couldn't find reference to them in the code, but the source is pretty complex so I could have missed the condition.

[tannewt]

We should call our internal close when the internal APIs indicate that the socket has been closed (by the other end or the lower layers.)

[hierophect]

I think I linked this by accident? #4095 is only tangentially related. I think I meant to link #4085?

[hierophect]

@brentru can you double check that this is fixed?

[brentru]

@hierophect Ok - I'll verify with MiniMQTT. Which CircuitPython version should I test on?

[hierophect]

@brentru latest. This should be all wrapped up with the new SSLSocket.

[brentru]

@hierophect this is fixed with the new SSLSocket

Tested with Adafruit CircuitPython 6.2.0-beta.2 on 2021-02-11; Adafruit MagTag with ESP32S2

[hierophect]

Sounds good, thanks for testing.