espressif/_bleio: build discovered GATT objects on the VM task

[dhalbert]

• Fixes BLE UART Write Fails with "Unknown system firmware error: 261" on ESP32-S3 #10748

This comment was mostly generated by Claude Code. Claude wrote most of the PR with considerable guidance and dialog with me. I used Opus 4.8 for the bulk of the PR, then as an experiment I asked Fable 5 to review the changes. It caught one additional problem.

Problem

On espressif, connecting as a BLE central and discovering a peripheral's services crashes partway through discovery with an internal watchdog reset (ESP_RST_INT_WDT → safe mode "Internal watchdog timer expired."). It reproduces with the Adafruit_CircuitPython_BLE ble_uart_echo_client.py example talking to a peripheral. It appears to be timing and storage sensitive: if the library is .py, not .mpy, or CircuitPython is built with -Os, the problem can go away. It has been present since before the ESP-IDF v6 migration.

Root cause

The remote GATT discovery callbacks (_discovered_service_cb(), _discovered_characteristic_cb(), _discovered_descriptor_cb()) are invoked by NimBLE on its host task (nimble_host), not the CircuitPython VM task (main). Both are pinned to core 1, with the host task at a much higher priority (21 vs 1). Those callbacks allocated CircuitPython objects (mp_obj_malloc(), common_hal_bleio_characteristic_construct(), mp_obj_list_append()) directly.

The CircuitPython espressif port has no GIL (MICROPY_PY_THREAD == 0) and assumes only the VM task touches the GC heap, so it is a mistake to do any gc heap allocation in other than the VM task. When an allocation on the host task triggers a gc_collect(), the collection runs on the NimBLE host task and gc_helper_collect_regs_and_stack() scans that task's stack rather than the VM task's, so the still-live, half-built discovery objects (rooted only on the VM task's stack) are treated as unreachable and freed. The resulting use-after-free corrupts the heap and takes the chip down. (Confirmed with temporary instrumentation logging the task inside gc_collect() — it printed nimble_host immediately before the reset.)

Fix

Keep all heap allocation on the VM task. The discovery callbacks now only copy the plain NimBLE result struct (ble_gatt_svc / ble_gatt_chr / ble_gatt_dsc — fixed-size PODs) into a fixed-size ring buffer; the VM task drains the ring and builds the Service / Characteristic / Descriptor objects. This mirrors the existing shared-module/_bleio/ScanResults.c pattern for BLE-callback to VM handoff.

The ring buffer's 4 KB backing store is allocated on first use from the port (IDF) heap via port_malloc() — not the GC heap — following the pattern of the port_malloc-backed ringbuf in shared-module/keypad/EventQueue.c. It therefore needs no GC root, lands in SPIRAM when available, survives soft reloads (so a stale procedure's late callbacks always write into live memory), and costs no RAM until a central actually discovers services. It is allocated once and never freed.

[Originally both LLM models suggested a static 4kB ring buffer, favoring it over gc heap allocation to avoid possible issues if BLE was still running but the VM had terminated. I suggested using port_malloc(), which overcame that worry. I tested on boards both with and without PSRAM to make sure it would be OK. --@dhalbert]

[The size of the ring buffer was calculated based on the possible traffic backlog during discovery. Both models agreed on the size, with Fable doing a more thorough calculation. --@dhalbert]

The ring buffer is guarded with common_hal_mcu_disable_interrupts() / enable_interrupts() around each whole-record put/get — and around the per-step ring reset — because ringbuf operations are not atomic (shared used counter) and the host and VM tasks preempt each other. The guarded region is a single small record copy, far below the interrupt-watchdog threshold.

Also in this PR: fixing some missing ringbuf atomicity guards in CharacteristicBuffer and PacketBuffer

A second commit guards the other _bleio state shared with the nimble_host task, which had the same latent race: the CharacteristicBuffer and PacketBuffer ringbuf operations, and PacketBuffer's pending outgoing buffers, which write() appends to on the VM task while queue_next_write() consumes them on the host task. Guard placement mirrors the nordic port's critical sections. An incorrect comment claiming the host task "won't interrupt us" has been removed.

Testing

Ran the ble_uart_echo_client.py example against a peripheral running ble_uart_echo_test.py on an nRF board, on three configurations: ESP32-S3 with 8 MB PSRAM, ESP32-S3 without PSRAM, and ESP32-C6. Also ran with the example programs swapped between Espressif and nRF. It works: discovery completes and the UART echo round-trips correctly on all three Espressif boards.

[tannewt]

Thanks for running down this bug. I think you should switch to using a freertos queue or similar instead of micropython's ringbuf. That way you'll get inter-task switching automatically instead of the polling this will do.

[dhalbert]

Thanks for running down this bug. I think you should switch to using a freertos queue or similar instead of micropython's ringbuf. That way you'll get inter-task switching automatically instead of the polling this will do.

Indeed, this came up as one of the alternatives with Claude Fable. See below, particularly the italicized (by me) section, which concluded the code still has to poll. @tannewt What do you think after reading the below?

Alternatives I considered, as a choice rather than a verdict:

• Keep as-is (ring + interrupt guard). Pro: identical to the ScanResults
  pattern, no new idioms, no dynamic allocation. Con: hand-rolled critical
  sections, 4 KB permanent .bss in every espressif BLE build for a
  discovery-only buffer, and correctness silently depends on the core-pinning
  config.
• FreeRTOS xQueue of fixed-size records. The idiomatic ESP-IDF answer:
  thread-safe regardless of core pinning, no manual interrupt games, and the VM
  side could still poll with zero timeout inside the RUN_BACKGROUND_TASKS loop.
  Con: introduces a second queueing idiom into _bleio, and the queue storage
  comes from the IDF heap.
• Heap-allocate the ring on the VM task at discovery start (like ScanResults'
  ringbuf_alloc), rooted via a local in discover_remote_services so GC keeps it
  alive. Reclaims the 4 KB when discovery isn't running; slightly fiddlier
  lifetime reasoning.

My take: the current shape is the right one for this codebase precisely
because it's the third instance of the ScanResults pattern — which strengthens
the case for the reusable guarded-ringbuf helper you already have queued as a
follow-up, rather than switching idioms in this PR. The only change I'd
actually consider before merging is guarding the ringbuf_init calls (issue 1).

[... discussed queue entry size here, not relevant to the decision]

❯ would it make the code a lot simpler or not really?

● Not really. The drain loop is where the complexity lives, and it survives
unchanged — you still need the timeout, the RUN_BACKGROUND_TASKS polling, the
interrupt check, the status check, and the final drain after status goes
nonzero. A queue doesn't let you block in xQueueReceive with a real timeout,
because the VM task must keep servicing background tasks and watching for
KeyboardInterrupt, so you'd still poll with a zero timeout inside the same
loop shape.

What actually changes:

• Deleted: _push_record and _pop_record (~25 lines of critical-section
  helpers) become bare xQueueSend(q, rec, 0) / xQueueReceive(q, &rec, 0) calls.
• Added: queue creation — either a StaticQueue_t control block plus a storage
  array in .bss with a one-time xQueueCreateStatic, or per-discovery
  xQueueCreate/vQueueDelete with an allocation-failure path. Either is a few
  lines you don't have today.
• Wash: the four ringbuf_init resets become four xQueueReset calls.

So it's roughly line-neutral. The honest case for the queue isn't simplicity,
it's robustness: xQueueSend/xQueueReset are thread-safe by construction, which
removes the hand-rolled critical sections, closes the unguarded-ringbuf_init
race (issue 1 from the review) for free, and stops the correctness from
depending on both tasks being pinned to the same core. The case against is
consistency: _bleio already speaks ringbuf-plus-interrupt-guard everywhere
else, and your planned guarded-ringbuf helper would get you most of the same
safety while keeping one idiom.

If the deciding question is "does it shrink the code," no. If it's "does it
delete the subtlest code," yes — the critical sections go away.