Address review feedback:
- Move error_str out of public UpdateInfo into file-local TaskResult
- Add container.reset() before vTaskDelete (doesn't call destructors)
- Remove redundant has_progress/progress assignments after move
- Add comment on delete after std::move
Address Copilot review feedback:
- Call container->end() when status_code != HTTP_STATUS_OK (pre-existing
bug, but easy to fix while we're here)
- Update error_str comment to say "update check failure" not "fetch failure"
- Move state computation to main loop callback (avoids 2 extra lambda captures)
- Add error_str field to UpdateInfo so error info rides with the pointer
- Allocate UpdateInfo once at top of fetch_manifest_ (single ownership path)
- Lambda captures only 2 pointers (8 bytes), fits std::function SBO
Reduces ESP8266 flash delta from +480 to ~+288 bytes.
The update task runs in a FreeRTOS thread on ESP32 and was writing
directly to update_info_ std::string fields while the main loop
reads them via API (as StringRef pointers into the string buffer),
MQTT, web server, and Prometheus. This is undefined behavior that
can cause use-after-free crashes when a string reallocation
invalidates a StringRef held by the API serialization path.
Move all update_info_ and state_ writes into the existing defer()
callback so they execute on the main loop. The task now accumulates
results in a local UpdateInfo struct and moves it into update_info_
in the deferred callback, eliminating the cross-thread data race
with zero steady-state memory overhead.
Move the fast-path checks from cleanup_(), process_to_add(), and
process_defer_queue_() into inline methods in the header. The common
case — nothing to clean up, nothing to add, nothing deferred — now
resolves with just an atomic load at the call site, avoiding a function
call into the .cpp translation unit entirely.
The slow paths remain in scheduler.cpp as cleanup_slow_path_(),
process_to_add_slow_path_(), and process_defer_queue_slow_path_().
Also moves the process_defer_queue_() body out of the header into
the .cpp file, reducing code bloat in every translation unit that
includes scheduler.h.
Add find_first parameter to cancel_item_locked_ and
mark_matching_items_removed_locked_, defaulting to false (cancel all).
When find_first=true, stops after the first match and exits across
containers. set_timer_common_ passes find_first=true where the
cancel-before-add invariant guarantees at most one match.
The public cancel path uses the default find_first=false to cancel ALL
matches, needed for DelayAction parallel mode where skip_cancel=true
can create multiple items with the same key.
Also fixes the Scheduler_SetInterval benchmark to call
process_to_add() every kKeyCount registrations with a final drain
after the inner loop, reflecting production behavior.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
The test was calling show_logs() which invokes _wait_for_serial_port("/dev/ttyUSB0").
Since that path doesn't exist in CI/dev environments, it spun in a 30-second polling
loop before returning. Add mock_wait_for_serial_port fixture to skip the wait.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
J. Nick Koston