Properly close socked which is already in disconnection state #150
Conversation
The socker might be already disconnecting due to FIN from the other side, in this case still close the socket to finish the operation but do not reset the ISR, because we will not get another SNIR_DISCON and would end up in inifite loop.
There was a problem hiding this comment.
The particulars of this change are beyond my knowledge.
I'm also not entirely sure how to correctly test it in a way that reproduces the issue that this change is resolving.
I did test the simpletest from this repo succesfully on this branch with a Feather RP2040 + Ethernet Featherwing
I also used this code to attempt to test the __exit__ context manager function:
import time
import board
import busio
import digitalio
import adafruit_connection_manager
import adafruit_requests
from adafruit_wiznet5k.adafruit_wiznet5k import WIZNET5K
print("Wiznet5k WebClient Test")
TEXT_URL = "http://wifitest.adafruit.com/testwifi/index.html"
JSON_URL = "http://api.coindesk.com/v1/bpi/currentprice/USD.json"
# For Adafruit Ethernet FeatherWing
cs = digitalio.DigitalInOut(board.D10)
# For Particle Ethernet FeatherWing
# cs = digitalio.DigitalInOut(board.D5)
spi_bus = busio.SPI(board.SCK, MOSI=board.MOSI, MISO=board.MISO)
# Initialize ethernet interface with DHCP
eth = WIZNET5K(spi_bus, cs)
pool = adafruit_connection_manager.get_radio_socketpool(eth)
conn_manager = adafruit_connection_manager.get_connection_manager(pool)
with conn_manager.get_socket("www.adafruit.com", 80, "http") as sock:
print(f"type(sock): {type(sock)}")
print(f"dir(sock): {dir(sock)}")
print("sleeping 10...")
time.sleep(10)
print("after")
Honestly I'm not entirely sure what outcomes are expected or not, but I can confirm it doesn't crash or seem to have any other adverse effects that I can tell.
@pinkavaj if you have one already, or know the proper way to write a short script that illustrates the issue and resolution if you can share that here it will be quite helpful.
There was a problem hiding this comment.
SNIR is the "socket n interrupt register", called Sn_IR in the datasheet. There are 5 interrupt bits defined in wiznet 5500, in bits 0-4. bits 5-7 are reserved. Writing a "1" bit clears the corresponding interrupt flag if it was set.
The change causes the call on that line to not clear the "disconnect interrupt", just every other kind of interrupt.
This change makes sense at least superficially, because the code is about to wait for the "disconnect interrupt" to occur before continuing.
Questions:
- is similar treatment needed for the timeout interrupt?
- the manufacturer recommended writing 0xff but does not wait for the socket to reach the closed state: https://docs.wiznet.io/Product/iEthernet/W5500/Application/tcp#socket-close -- is there something incorrect about the whole idea of waiting for the interrupt flag? It's also possible to poll the status register for the socket entering the closed state, separate from checking the interrupt flags.
Updating https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k to 6.0.0 from 5.2.1: > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#150 from pinkavaj/pi-fix-sock-exit > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#160 from justmobilize/remove-wsgiserver Updating https://github.com/adafruit/Adafruit_CircuitPython_ConnectionManager to 1.2.0 from 1.1.0: > Merge pull request adafruit/Adafruit_CircuitPython_ConnectionManager#13 from justmobilize/close-all-and-counts Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 3.2.6 from 3.2.5: > Merge pull request adafruit/Adafruit_CircuitPython_Requests#188 from justmobilize/with-updates Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Updated download stats for the libraries
The socker might be already disconnecting due to FIN from the other side, in this case still close the socket to finish the operation but do not reset the ISR, because we will not get another SNIR_DISCON and would end up in inifite loop.