F-26: OrchestratorClient::m_client destroyed under a concurrent sender #29

Severity: MEDIUM · Effort: Medium · Batch: 4 · Status: Code done in #21 (1b8767b), awaiting bench verification

Found in the second full review of the firmware. Recorded with the rest of the pass (F-19…F-44) in docs/REVIEW_REMEDIATION_PLAN_2.md, which is where its status is kept.

Description

connect() calls disconnect(), which stops and destroys the WebSocket handle, while
sendJson on the encoder task dereferences m_client without a lock. Pressing Connect on the
orchestrator screen while driving — or the screen's connect racing the boot orch_connect task
— is a use-after-free inside esp_websocket_client_send_text. The "already connecting" check is
also check-then-act, so two connect tasks can both pass it.

Plan

Guard the handle with its own mutex held across send and across stop/destroy (sends are
bounded at 1 s), and make the connecting check atomic.

Progress: Batch 3 made the orch_connect supervisor the only caller of connect() — the
config screen now wakes it rather than connecting on a task of its own — so two connects can
no longer race. Batch 4 added the handle mutex, so a re-login can no longer free the handle
under a sender.

Files: OrchestratorClient.h/.cpp

Acceptance Criteria

• No path reads m_client without the handle mutex: sends take it (bounded, and only
  once the link is known up), openSocket publishes under it, and disconnect() holds it
  across stop and destroy.
• The connecting check need not be atomic any more: the supervisor is the only caller of
  connect() (batch 3).
• Bench: press Connect repeatedly while spinning a knob; no crash.