mirror of
https://github.com/agessaman/MeshCore.git
synced 2026-10-20 12:22:13 +00:00
agessaman
2d4490bcaa
The timeout fallback did the two things that cannot be done safely to a task that may still be inside mbedTLS or holding esp-mqtt's API mutex: it deleted that task from the loop task, then force-stopped and deleted the clients it owned, then freed the queue and buffers it can reach. `esp_mqtt_client_stop()` waits on the API mutex and the task's stopped event with no bound, so killing its caller strands the next one; a second stop sees the run flag already clear and returns failure without joining. Nothing in that path established quiescence, and `canFlashAfterStop()` could only withhold OTA afterwards. Changes, all of them about who owns what: - `MQTTLifecycle` gains a `StopUnproven` state (appended, so existing values keep their numbering). `StopTimedOut` now leads there and releases NOTHING; it still fires `ota_release` so the OTA barrier aborts. `acceptsNewWork`, `mayRestart` and `isStopInProgress` are false there, `mayTouchOwnedState` is true, and `end()`'s wait loop still terminates. - `begin()` refuses to start while a stop is unproven. A new start was never proof that the previous clients stopped, and it used to clear the dirty latch that withheld OTA. Because a start is now impossible from `StopUnproven`, clearing that latch on an accepted start is sound. - A late acknowledgement is honoured. The task publishes it only after tearing its clients down, so it proves the same thing whenever it arrives; the deadline is an availability policy, not a statement about what the ack means. `pollLateStopAck()` releases the withheld resources and makes the bridge restartable — a slow stop (a blackholed WSS broker can hold the SDK well past the budget) no longer costs a reboot. - The handshake flags become `std::atomic<bool>` with release/acquire, and the ack moves into the task trampoline immediately before `vTaskDelete(nullptr)`, gated on a `_teardown_complete` flag the loop sets. Published from inside the loop it proved teardown returned, not that the task had stopped executing — and `volatile` ordered none of it. This is the soak campaign's blocker #20, the one item on its list that can corrupt memory. - The force-stop paths are gone with their only caller: `destroySlotClients()` and `teardownSlot()` no longer take a `force` flag. - `get mqtt.status` reports "previous stop unproven, reboot to recover" instead of a bare "not running", latched so it survives end() clearing the singleton. Two lifecycle tests asserted the old contract and are re-encoded, not deleted: a timed-out stop is no longer `Stopped`-and-released, and "repeated failed stops leave the bridge restartable" becomes "repeated *slow* stops recover on their late ack" plus a new case pinning that a never-acknowledged stop stays unusable for the boot. That availability trade is the point of the change.