Skip to content

fix: avoid UiFtpServer mutex crash at boot - #448

Merged
mverch67 merged 3 commits into
meshtastic:masterfrom
alecperkins:fix/ftp-server-startup-race
Oct 30, 2026
Merged

mverch67 merged 3 commits into
meshtastic:masterfrom
alecperkins:fix/ftp-server-startup-race

Conversation

@alecperkins

@alecperkins alecperkins commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Avoid taking a lock inside isWiFiConnected() which can cause the device to crash while the wifi is still being started.

Tested on Elecrow M9.

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability during FTP server initialization and Wi-Fi connectivity checks, particularly when these operations occur alongside other activity. These changes do not add or remove user-facing features, settings, or controls. The FTP server and Wi-Fi status should continue to behave as before, with more consistent operation in concurrent situations.

The UI polls isWiFiConnected() every second from boot on, even when FTP
was never started. Its lock was the first use of the class mutex, and
ESP-IDF allocates a std::mutex lazily on first lock. While WiFi was
starting up this crashed the ThinkNode M9 in 3 of 4 boots (abort on
system_error, interrupt WDT, or LoadProhibited inside the lock).

isWiFiConnected() reads no class state, so drop the lock, matching
RSSI(). Also make instance() creation thread-safe, since the WiFi event
handler and the FTP server task call it too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 8dde4ec and 499daad.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 0d400e2 and 58191f4.

Files selected for processing: 1
  • source/comms/UiFtpServer.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

Walkthrough

UiFtpServer::instance() now uses function-local static initialization. isWiFiConnected() checks WiFi status without locking the class mutex.

Changes: FTP server synchronization

Layer / File(s) Summary
Singleton and WiFi status checks
source/comms/UiFtpServer.cpp
instance() uses a function-local static pointer. isWiFiConnected() checks WiFi status without acquiring the class mutex.

Priority: Normal

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: mverch67

Merge Risk: Minimal · previous review 58191

No actionable issue is established with the FTP synchronization change; it is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: preventing a UiFtpServer mutex-related crash during boot. It matches the changes and objectives.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@CLAassistant

CLAassistant commented Oct 24, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@mverch67
mverch67 merged commit 6034d76 into meshtastic:master Oct 30, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants