-
Notifications
You must be signed in to change notification settings - Fork 31
Fix infinite loop when requesting too quickly #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
On the Pico W we can get in an infinite loop if we request pages too quickly. This is because we get an OSError 32 Broken Pipe back from which we can't recover. I've made it so any of the unexpected errors will be re-raised. Fixes this issue: adafruit#44
Fix linting issue
Ensure unexpected OSError isn't swallowed.
Whilst I can't re-create for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems okay to me. I tested on a a PicoW with 8.1 beta. Using the currently released version I did end up in the infinite loop / locked up state once by spam clicking the mute button on the index page.
Using the version from this PR I have not gotten back into that infinite loop state. All of my requests are succeeding, I spam clicked many times and even ran a python script with a tight loop making requests over and over the PicoW was able to handle all requests and return proper responses. I'm not certain if I just got "lucky" with the testing, but I never did have a raised error stop my server poll from continuing.
I don't necessarily understand the potential ramifications in the core, like is this really specific to picow, and if so is there a fix in the core that would be better, or should go alongisde this?
I'll wait for others to chime in on that, but if there is nothing further that needs done on that front then this seems good to me and I'll come back to approve if not.
Thanks for testing.
Yeah it's worth understanding if the Broken Pipe is due to some other issue that needs to be fixed. That being said catching OSError - and then checking for a couple of specific errors and swallowing any other error is at worst hiding issues but at best dangerous of getting in this infinite loop - so I think this change should still be done in case of any other OSErrors that may be thrown in the future. |
@FoamyGuy Shall we merge this too, including it in all the new changes? |
@dhalbert Yep sounds good, I'll merge this one as well. I made new release with the change from the other PR before I saw you're note here. But will make another new one after this merge. |
Updating https://github.com/adafruit/Adafruit_CircuitPython_AGS02MA to 1.0.4 from 1.0.3: > Merge pull request adafruit/Adafruit_CircuitPython_AGS02MA#3 from jposada202020/docs > Add upload url to release action > Add .venv to .gitignore > Update .pylintrc for v2.15.5 > Fix release CI files > Update pylint to 2.15.5 > Updated pylint version to 2.13.0 > Switching to composite actions Updating https://github.com/adafruit/Adafruit_CircuitPython_CLUE to 3.0.14 from 3.0.13: > Merge pull request adafruit/Adafruit_CircuitPython_CLUE#59 from julianaklulo/main > Add upload url to release action > Add .venv to .gitignore > Update .pylintrc for v2.15.5 > Fix release CI files > Update pylint to 2.15.5 > Updated pylint version to 2.13.0 > Switching to composite actions Updating https://github.com/adafruit/Adafruit_CircuitPython_EPD to 2.11.0 from 2.10.8: > Merge pull request adafruit/Adafruit_CircuitPython_EPD#61 from ladyada/master > Add upload url to release action > Add .venv to .gitignore > Update .pylintrc for v2.15.5 > Fix release CI files > Update pylint to 2.15.5 > Updated pylint version to 2.13.0 > Switching to composite actions Updating https://github.com/adafruit/Adafruit_CircuitPython_MCP2515 to 1.1.1 from 1.1.0: > Merge pull request adafruit/Adafruit_CircuitPython_MCP2515#18 from adafruit/use-core-message-rtr-classes Updating https://github.com/adafruit/Adafruit_CircuitPython_framebuf to 1.6.1 from 1.6.0: > Merge pull request adafruit/Adafruit_CircuitPython_framebuf#52 from steka/improve_font_to_bin Updating https://github.com/adafruit/Adafruit_CircuitPython_HTTPServer to 3.0.1 from 2.5.0: > Merge pull request adafruit/Adafruit_CircuitPython_HTTPServer#45 from foxy82/main > Merge pull request adafruit/Adafruit_CircuitPython_HTTPServer#49 from michalpokusa/prevent-parent-directory-access Updating https://github.com/adafruit/Adafruit_CircuitPython_Pixel_Framebuf to 1.1.10 from 1.1.9: > Merge pull request adafruit/Adafruit_CircuitPython_Pixel_Framebuf#9 from RossK1/adding_type_hints > Add upload url to release action > Add .venv to .gitignore > Update .pylintrc for v2.15.5 > Fix release CI files > Update pylint to 2.15.5 > Updated pylint version to 2.13.0 > Switching to composite actions Updating https://github.com/adafruit/Adafruit_CircuitPython_TinyLoRa to 2.2.13 from 2.2.12: > Merge pull request adafruit/Adafruit_CircuitPython_TinyLoRa#49 from awordforthat/issue47/fix-short-name-errors > Add upload url to release action > Add .venv to .gitignore Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Updated download stats for the libraries
On the Pico W we can get in an infinite loop if we request pages too quickly. This is because we get an OSError 32 Broken Pipe back from which we can't recover. I've made it so any of the unexpected errors will be re-raised.
Fixes this issue:
#44
I also wonder if the same issue can happen in
server._receive_header_bytes
/server._receive_body_bytes
which has very similar code that catches and swallows OSError unless it is ETIMEDOUT ?