supervisor/web_workflow: check the listener socket calls for failure - #11207
Open
mikeysklar wants to merge 1 commit into
Open
supervisor/web_workflow: check the listener socket calls for failure#11207mikeysklar wants to merge 1 commit into
mikeysklar wants to merge 1 commit into
Conversation
start_web_workflow() created the listening socket, bound it and called
listen() without checking any of them, with an explicit "(Not checking for
failures)" comment on the bind. On a port whose network stack can refuse to
create a socket, the descriptor stays unset and bind() and listen() then run
on -1 and fail with EBADF, but the function still returns true. The caller
takes that as success and installs the background callback, so the supervisor
polls a socket that was never opened and the board can hang before running
code.py. Nothing is ever served, and there is no diagnostic.
Observed on a Silicon Labs SiWx917-DK2605A (BRD2605A) running the zephyr-cp
port, CircuitPython 10.3.0-alpha.4, where socket() returns ENOTCONN until the
interface has an address:
socket() ok=0 num=-1 errno=107 ENOTCONN
bind port 80 -> 9 errno=9 EBADF
listen -> 0 errno=9 closed=1
(returns true)
Return false instead of reporting success, and close the socket if bind or
listen is the step that failed. Note this is not a retry: the next attempt is
the next supervisor_workflow_reset(). Bringing the workflow up once the
interface acquires an address later is a separate change.
This is not specific to that port. adafruit#10054 reports web workflow unreachable
after a watchdog reset on several ESP32 boards, and the attempted fix in
adafruit#10948 moved port 80 from closed to filtered, i.e. bound but never serving,
which is the same end state this produces.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
supervisor_start_web_workflow()created the listening socket, bound it and calledlisten()without checking any of them, with an explicit// Bind to any ip. (Not checking for failures)comment on the bind. On a port whose network stack can refuse to create a socket, the descriptor stays unset,bind()andlisten()run on -1 and fail withEBADF, and the function still returns true.The caller takes that as success and installs the background callback, so the supervisor polls a socket that was never opened. Nothing is served and there is no diagnostic.
Why
This is shared supervisor code, so it affects every port that has web workflow: espressif, raspberrypi and zephyr-cp.
CIRCUITPY_WEB_WORKFLOW ?= $(CIRCUITPY_WIFI), so atmel-samd, stm and nordic do not compile this file and are unaffected.Not specific to the port where it was found. #10054 reports web workflow unreachable after a watchdog reset on several ESP32 boards, and the attempted fix in #10948 moved port 80 from closed to filtered, that is bound but never serving, which is the same end state this produces.
Hardware tested
Two parts: the original failure on zephyr-cp, and a no regression check on the two ports that already ship web workflow.
Failure case
Silicon Labs SiWx917-DK2605A (BRD2605A), zephyr-cp port, host on the same LAN. Serial output before the change:
With the change the workflow either starts or reports failure, rather than reporting success on a socket that was never opened. Verified serving afterwards:
/cp/version.json200,/fs/401 without credentials and 200 with, PUT 201, GET 200, DELETE 204.No regression on espressif and raspberrypi
Three boards flashed with this patch, blank
CIRCUITPY_WEB_API_PASSWORD, all on the same LAN. REPL banners:/cp/version.json/cp/devices.json/fs/401 on
/fs/is the expected response with a blank password.Beyond the scripted checks above, I opened the web workflow page on each of the three boards from a browser on the same LAN, running the build in those banners, and confirmed it loads and serves. No regression on either port.
Also built for
renesas_ek_ra8d1, a zephyr-cp board CI builds today.The
-dirtysuffix is a build host artifact.ports/espressif/esp-idfis a symlink to a shared checkout at the pinned commit2b900d12, which git reports as a typechange. No source file is modified.Scope
Error checking only. Deliberately not a retry: the next attempt is the next
supervisor_workflow_reset(). Bringing the workflow up once the interface acquires an address later is a separate change and is not attempted here.AI assistance
Written with Claude Code. I ran the hardware myself and checked the web workflow by hand in a browser on all three boards. The serial output, REPL banners and HTTP results above are from the boards, not from a model.