adafruit / Adafruit_CircuitPython_Wiznet5k

Pure-Python interface for WIZNET 5k Ethernet modules
Other
15 stars 35 forks source link

Hardware socket incorrectly reserved in socket.accept() when socket 0 is available. #129

Closed fasteddy516 closed 1 year ago

fasteddy516 commented 1 year ago

When hardware socket 0 is not in use (as is the case when DHCP is not enabled), accepting connections on a listening socket results in an unused socket being incorrectly tagged as 'reserved', blocking it from ever being used.

I wrote a script to help troubleshoot this (and other) socket issues, which is available at https://github.com/fasteddy516/CircuitPython_Wiznet5k_Socket_Visualizer. The output is below:

socket_visual

The server starts out listening on socket 1. The first accepted connection gets assigned to socket 1, and the server starts listening on socket 0 but reserves socket 2, effectively blocking it from ever being used.

I've tracked the issue down to the socket swap that happens in socket.accept(), specifically here:

new_listen_socknum, addr = _the_interface.socket_accept(self._socknum)
current_socknum = self._socknum
# Create a new socket object and swap socket nums, so we can continue listening
client_sock = socket()
client_sock._socknum = current_socknum  # pylint: disable=protected-access
self._socknum = new_listen_socknum

I believe I've fixed the issue by ignoring the next_socknum returned by socket_accept() and using the socket number originally assigned to the new socket instead:

_, addr = _the_interface.socket_accept(self._socknum)
current_socknum = self._socknum
# Create a new socket object and swap socket nums, so we can continue listening
client_sock = socket()
self._socknum = client_sock._socknum
client_sock._socknum = current_socknum  # pylint: disable=protected-access

Note that this prevents hardware socket 0 from being used to listen for incoming connections, but I think that's how it's supposed to work anyway.

It also looks like next_socknum can be stripped completely out of WIZNET5K.socket_accept() as the only place it was used was in socket.accept(), but that changes the API and I wasn't sure if that would be problematic.