nrf52: fix BLE-task stack overflow crashing pairing and wiping LittleFS - #11190
Conversation
Since #10967 made Router::sendLocal handle self-addressed packets synchronously, the entire phone-API chain for a BLE client runs inline in the Bluefruit characteristic write callback: toRadioWriteCb -> PhoneAPI::handleToRadio -> admin set-config -> radio reconfigure -> NodeDB::saveToDisk. That callback executes on the Bluefruit BLE FreeRTOS task, whose stock stack is 5 KB (CFG_BLE_TASK_STACKSIZE = 256*5 words) - not the Arduino loop task that #10944 already raised to 8 KB. The loop-task fix therefore protects the wrong task for BLE-originated writes. On a Seeed Wio Tracker L1 the 5 KB stack overflows during pairing first-sync, resetting the device mid-LittleFS-write, every single time. Repeated mid-write resets tear the LittleFS metadata, lfs_assert fires on the next boot, and the corruption handler formats the whole filesystem: region, channels, module config, and the node's keypair are all lost (critical fault #13, new node identity on next region set). Reproduced end-to-end tonight on stock develop 6908d27; with this change the same device pairs, serves config screens, and survives back-to-back config.proto saves over BLE. Raise the BLE task to the same 2048 words (8 KB) as LOOP_STACK_SZ, for the same reason. bluefruit.cpp's #ifndef guard makes the -D take effect with no framework patch. Costs 3 KB of RAM on nrf52840 targets only. Credit where due: Ixitxachitl independently established in #11155 testing that the save-path crash persists after #11185 and that re-queueing sendLocal (moving the pipeline back to the Router thread) makes it go away - which corroborates this diagnosis from the other direction. This commit is the minimal capacity-side fix; #11155's relocation of the pipeline off the BLE task remains the right architectural follow-up, and this guard stays correct even after it lands. Likely also explains #10905 (L1 display-thread crash when a client requests full configuration) and the 2.8 field reports of idle nodes losing region and keys after a BLE session.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe nRF52840 build configuration adds ChangesBLE stack configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
⚡ Try this PR in the Web FlasherNote Building this pull request… the flash button, badges and supported-board |
The bug
On nRF52 targets, BLE pairing/first-sync can crash the device the moment the client writes config — and repeated crashes escalate to a total filesystem wipe: LittleFS metadata gets torn by the mid-write reset,
lfs_assertfires on the next boot, the corruption handler formats the FS, and the node loses region, channels, module config, and its keypair (critical fault #13, new identity once region is set again).Reproduced 100% on a Seeed Wio Tracker L1 on stock develop
6908d27: every iPhone pairing attempt reset the device atOpening /prefs/config.proto, fullAtomic=1, and the wipe cascade followed. This is very likely also #10905 (L1 display-thread crash on full config request) and a match for 2.8 field reports of nodes losing region/keys after a BLE session.Root cause
Since #10967 made
Router::sendLocal()handle self-addressed packets synchronously, the entire phone-API chain runs inline in the Bluefruit characteristic write callback:That callback executes on the Bluefruit BLE FreeRTOS task, whose stock stack is 5 KB (
CFG_BLE_TASK_STACKSIZE = 256*5words). #10944 raised the Arduino loop task to 8 KB for exactly this chain — but after #10967, BLE-originated writes no longer run there, so the fix protects the wrong task. Measured frames (-fstack-usage, seeed_wio_tracker_L1):handleToRadio520 B +handleReceivedProtobuf584 B +handleSetConfig520 B +perhapsDecode472 B + ~450 B of glue ≈ 2.5 KB of application frames before LittleFS and the SX126x reconfigure even start — a 5 KB task can't carry it.The fix
-DCFG_BLE_TASK_STACKSIZE=2048(words = 8 KB) invariants/nrf52840/nrf52.ini, right next to the existingLOOP_STACK_SZ=2048and for the same reason.bluefruit.cppguards the define with#ifndef, so it takes effect without a framework patch. Costs 3 KB RAM, nrf52840 targets only.Validation
A/B on the Wio Tracker L1, same commit, only this flag differing:
set_configdevice + LoRa/region (twoconfig.protosaves over BLE)The 8 KB constant was verified in the shipped ELF (
mov.w r2, #2048intoxTaskCreateinBluefruit::begin). rak4631 and seeed_wio_tracker_L1 both build green. Native suites unaffected (build-flag change only).Credit and relationship to #11155
@Ixitxachitl independently established during #11155 testing that the save-path crash persists after #11185 and that re-queueing
sendLocal(which moves this pipeline back to the Router thread, i.e. the 8 KB loop task) makes it disappear — corroborating this diagnosis from the other direction. This PR is the minimal capacity-side guard that stops the crash-and-wipe today; #11155's relocation of the pipeline off the BLE task is the right architectural follow-up, and this guard remains correct after it lands (the BLE task still services SoftDevice events and other callback work, and 5 KB was marginal for that regardless).🤖 Generated with Claude Code
Summary by CodeRabbit