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, with edits and additions by @dhalbert. 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 FreeRTOS queue; the VM task drains the queue and builds the Service / Characteristic / Descriptor objects.

The queue is created on first use via xQueueCreate(), from the FreeRTOS (IDF) heap — not the GC heap. It therefore needs no GC root, 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 created once and never deleted.

The size of the queue was calculated based on the possible traffic backlog during discovery. Both models agreed on the size, with Fable doing a more thorough calculation.

The queue operations (xQueueSend(), xQueueReceive(), and the per-step xQueueReset()) are thread-safe by construction, so no explicit interrupt guarding is needed for the discovery staging.

The fix originally used ringbuf.h instead of an xQueue; @tannewt requested using the native FreeRTOS mechanism in his review. The newer code is a little simpler.

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.

After the change from ring buffer to FreeRTOS queue, retested on ESP32-S3 and nRF with the echo examples, in both role arrangements.

[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.

[tannewt]

It's not taking into account my desire to move to an RTOS world in the future where background tasks can be done by separate tasks.

I do also think it simplifies the locking and memory allocation story a bit too.

So, I disagree, for code within the ESP or Zephyr port we should use the RTOS primitive when we can. ringbuf can still be used for shared code though.

[dhalbert]

So, I disagree, for code within the ESP or Zephyr port we should use the RTOS primitive when we can. ringbuf can still be used for shared code though.

That is fine, I will change it. I don't disagree with you. I was more noting that we can't avoid the polling. But it does make the code a little simpler, since the xQueue takes care of the storage management and is a better abstraction. Right now the ringbuf is ad hoc message passing, and touchy storage management.

[dhalbert]

@tannewt I switched to xQueue - ready for re-review