[1.4] core: services: ardupilot_manager: Cap consecutive start failures - #4121
Open
joaoantoniocardoso wants to merge 1 commit into
Open
Conversation
Automated PR Review0. Summary
Caps No further comments, nice job 👍 Generated by PR Review Bot. This is advisory, a human reviewer must still approve. |
Merged
2 tasks
When no board can be started, auto_restart_ardupilot() retried every 5s forever. Each cycle leaks file descriptors, so after ~30 minutes the service hit EMFILE and uvicorn's accept loop began logging a traceback per retry, pegging the CPU and making the API unresponsive. That also removed the only recovery path, since selecting SITL requires a working /available_boards. Give up after 10 consecutive failures instead. Transient failures still self-heal, and an explicit start or board change re-arms the watchdog. The counter lives in __init__ rather than setup(), because setup() runs on every start attempt and would reset it. Fixes bluerobotics#4070
joaoantoniocardoso
force-pushed
the
fix/1.4-autopilot-start-failure-cap
branch
from
August 11, 2026 21:13
2369af6 to
9a86718
Compare
This was referenced Aug 11, 2026
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.
Relates to #4070. Rebased onto 1.4-dev so it sits on top of 71500f9 (
fix(i2c): prevent file descriptor leak in SMBus usage), which matters for the story below.Problem
A customer moved an SD card from a Pi 4 with a Navigator to a Pi 4 without one. The autopilot service pegged the CPU, became unresponsive, and recovered only when the card went back to the original Pi.
From the logs of that incident, in
auto_restart_ardupilot():get_board_to_be_used()refuses to start:Only available board is SITL, and it wasn't explicitly chosen.GET /available_boardsstill returns 200OSError: [Errno 24] Too many open filesGET /available_boardsreturns 500socket.accept()logs a full traceback per retry, forever, ~150MB in ten minutesTwo things went wrong, and the second was a consequence of the first:
available_boards, but that endpoint started returning 500 once descriptors ran out, and the frontend clears its board list on error, leaving an empty "Available boards" dropdown.POST /boardcallsavailable_boards()too, so it would have failed as well.What actually exhausted the descriptors
I reproduced this on a Pi 4 running this branch, by booting a Navigator-configured SD card with the HAT removed. The service settled at 338 open descriptors against a soft limit of 1024, of which 330 were handles on
/dev/i2c-1— about 32 leaked per retry cycle.That arithmetic matches the incident exactly: at 32 per cycle the limit arrives at ~31 cycles, and the customer hit EMFILE after 32.
The leak was
check_for_i2c_device()callingSMBus(bus_number)without ever closing it. With a HAT present, detection succeeds on the first pass and is cached, so it leaks 4 descriptors once and nobody notices. With the HAT absent, detection never caches, so all 5 passes re-probe on every call and every probe leaks.That leak is already fixed on 1.4-dev by 71500f9, which this PR is rebased on. I verified the fixed version leaks nothing: 200 probes against the Navigator's four addresses on a bare Pi, all failing with
[Errno 121] Remote I/O error, zero descriptor growth.So 71500f9 is what prevents the EMFILE storm and keeps the API responsive. This PR is no longer the root-cause fix, and the remaining justification is narrower.
What this PR still fixes
With the leak fixed, a Pi that cannot start any board still retries forever, every 5 seconds, indefinitely. Each cycle runs a full
kill_ardupilot()(including a MAVLink disarm attempt that times out), tears down and recreates theMavlinkManager, and runs a 5-pass i2c detection sweep.Measured on hardware: ~8KB of logs per cycle, ~12 cycles/minute, so roughly 140MB/day written to the SD card, forever, for a machine that will never succeed without user action.
This gives up after 10 consecutive failures instead:
POST /startorPOST /boardre-arms the watchdog, sincestart_ardupilot()still setsshould_be_running = Truein itsfinally. That gives one further attempt per user action.__init__rather thansetup(), becausesetup()runs on every start attempt and would reset it.It also bounds the blast radius of any future resource leak on this path, which is how the original incident escalated from "no board" to "unrecoverable".
Hardware validation
Pi 4, this branch, Navigator-configured SD card, HAT physically removed,
preferred_boardunset (matching the customer's settings).Consecutive start failures threshold reached, ~2m45s after bootNote the 338 descriptors above were measured on a build predating the rebase, so without 71500f9. With both changes the i2c descriptors should stay flat.
Considered and deliberately left out
should_be_running = Trueout of thefinallyinstart_ardupilot(). Smaller diff, larger behaviour change: it removes auto-retry for transient failures entirely and flipsis_running()for serial boards. Worth doing on its own merits, not in a release-candidate branch.POST /boarddepends on the same failing endpoint.Known limitation, not addressed here
"Change board", "Start autopilot", and "Stop autopilot" are all gated behind pirate mode in
core/frontend/src/views/Autopilot.vue. After the cap trips, support and pirate-mode users can select SITL, but a regular user has no recovery path in the UI. That is pre-existing behaviour, unrelated to this incident, and worth discussing separately.Test plan
pytestoutput matches the base commit exactly, including a pre-existing localardupilot_fw_decodercollection error and 74.32% coverage