adafruit / Adafruit_CircuitPython_Wiznet5k

Pure-Python interface for WIZNET 5k Ethernet modules
Other
16 stars 36 forks source link

.socket() allows creation of more sockets than available #93

Closed anecdata closed 1 year ago

anecdata commented 1 year ago

socket.socket() correctly fails with RuntimeError: Failed to allocate socket. after creating all available sockets (4 for 5100S, 8 for 5500):

socks = []
while True:
    s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
    socks.append(s)
    print(f"{s} {s.sendto(b'\xFF', (HOST, PORT))}")
    time.sleep(1)

socket.socket() will create sockets forever:

socks = []
while True:
    s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
    socks.append(s)
    print(f"{s}")
    time.sleep(1)

On native wifi implementations, the latter example will fail with RuntimeError: Out of sockets after creating all available sockets.

The library could flag the error at creation time, rather than at time of (attempted) use.

anecdata commented 1 year ago

I don't really know what the proper behavior should be. I don't find any guidance from CPython where resources aren't constrained like this. Maybe it's OK to initialize too many sockets, and just not use too many simultaneously.

The main issue I think is that the behavior differs between different implementations within CircuitPython. ESP32SPI also currently behaves like this library. NINA allows 10 sockets, but more can be initialized as long as they are not used.

BiffoBear commented 1 year ago

I don't really know what the proper behavior should be. I don't find any guidance from CPython where resources aren't constrained like this. Maybe it's OK to initialize too many sockets, and just not use too many simultaneously.

The main issue I think is that the behavior differs between different implementations within CircuitPython. ESP32SPI also currently behaves like this library. NINA allows 10 sockets, but more can be initialized as long as they are not used.

It does seem wrong to allow resources to be assigned that can never be used. If a program creates n instances of a socket, it expects to be able to use them. If the Wiznet5K driver allows unlimited instances of socket.socket, then tries to juggle them with the limited number of sockets available then some very strange behaviour is likely to occur. This would most likely be difficult to debug.

The simplest solution is to do nothing, the next simplest is to keep track of the hardware sockets that are linked to instances of socket.socket. I've issued a PR that does this. The caveat is that it relies on the programmer to either use a with statement or call socket.close() because Python's garbage collector doesn't guarantee when the instances will be destroyed. Also, one socket is reserved so that DNS lookups, etc. can take place when hardware sockets are assigned to socket.socket. This leaves 3 sockets on a W5100s' and 7 on aW5500`

Kind of a least worst scenario.

<ASSUMPTION WARNING!> Although there is apparently no limit on socket instances in C Python, the underlying OS will have a limit. A quick check on stack exchange suggests that the OS integer size is a limit and it's different from OS to OS. So if C Python is wrapping the OS's socket implementation, I expect that C Python will pass on the exception if it ever reaches the OS limit.