feat(websocket): Add non-blocking stop request and bounded stop wait - #1122
Draft
wonderdog5 wants to merge 1 commit into
Draft
feat(websocket): Add non-blocking stop request and bounded stop wait#1122wonderdog5 wants to merge 1 commit into
wonderdog5 wants to merge 1 commit into
Conversation
esp_websocket_client_stop() (and destroy(), which calls it) joins the client task with portMAX_DELAY. A caller that must not block for long — e.g. a power-management path with a fixed sleep-readiness deadline — has no way to bound that join: with the task mid-reconnect, the join can stack the TLS/transport connect timeout on top of the wait, and tearing down two clients back to back doubles it. Split the stop into its two halves: - esp_websocket_client_request_stop(): flag the task to exit and wake a reconnect wait early (REQUESTED_STOP_BIT), return immediately. - esp_websocket_client_wait_stopped(): wait for STOPPED_BIT with a caller-chosen timeout; only on success is destroy() guaranteed not to block on the task. Requesting stop on several clients before waiting on any overlaps their teardowns instead of serializing them. esp_websocket_client_stop() behavior is unchanged (request + portMAX_DELAY wait). Co-Authored-By: Claude Fable 5 <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
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.
Problem
esp_websocket_client_stop()— andesp_websocket_client_destroy(), which calls it — joins the client task withportMAX_DELAY. That is fine for callers that can block, but there is a real deadlock-adjacent class it creates: a task that must not block unboundedly has no bounded way to tear a client down.Concrete case (ESP32-S3 battery device, two websocket clients): a power-management task with a fixed sleep-readiness deadline tears down both clients back to back on the way into deep sleep. A link blip is exactly when sleep tends to get requested, so both client tasks are typically mid-reconnect — each join then waits out the full TLS/transport connect timeout before the task notices
run == false, and the two unbounded joins stack. With a ~15 s connect timeout per client, the teardown can exceed a 30 s system deadline, and the caller can neither cancel nor bound it.Change
Split the stop into the two halves it is already made of internally:
esp_websocket_client_request_stop()— the non-blocking half: setsrun = falseandREQUESTED_STOP_BIT(which also wakes aWEBSOCKET_STATE_WAIT_TIMEOUTreconnect wait early, so a task idling between retries exits immediately), then returns.esp_websocket_client_wait_stopped(client, timeout)— a bounded wait onSTOPPED_BIT. Only after it returnstrueisesp_websocket_client_destroy()guaranteed not to block on the task; onfalsethe caller keeps the handle and retries later.An application tearing down several clients can request all stops first — the task exits overlap — and then wait for each against a single shared deadline, instead of serializing every client's worst-case teardown.
esp_websocket_client_stop()is unchanged (still request + unbounded wait); no existing behavior is affected. No new state or bits are introduced — the split only exposes the existingREQUESTED_STOP_BIT/STOPPED_BITmachinery with a caller-chosen timeout.Notes
request_stopmirrorsstop_wait_task's guard: it cannot be called from the websocket task itself, and is a no-opESP_OKwhen already stopped.wait_stopped(portMAX_DELAY)reproduces the current unbounded join exactly.I'm happy to adjust naming, add a test under
components/esp_websocket_client/tests, or fold this into a different API shape if the maintainers prefer.🤖 Generated with Claude Code