Skip to content

fix(l2cap): serialize writes and drain channel lifecycle work - #449

Draft
mickeyl wants to merge 5 commits into
h2zero:masterfrom
mickeyl:upstream/l2cap-lifecycle
Draft

mickeyl wants to merge 5 commits into
h2zero:masterfrom
mickeyl:upstream/l2cap-lifecycle

Conversation

@mickeyl

@mickeyl mickeyl commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #448. Keep this draft until that correctness fix lands, then rebase. The lifecycle-only changes can be reviewed here: mickeyl/esp-nimble-cpp@25a9994...8bba853

Concurrent writers, early credit completions and disconnect/deinit can leave send waiters or callback work referring to an old connection. Serialize whole writes, submit native operations on the host, retain completion state across early notifications, abort incomplete writes and drain pending work before channel reuse/destruction. Reclaim the shared callback worker/queue after the last channel, and delete the CoC server before the native NPL runtime is released.

The pool now allocates configurable full-SDU buffers, including mbuf and packet-header headroom. This changes memory use: five buffers by default, rather than subdividing about three MTUs into 250-byte blocks.

Observable callback contract:

  • onConnect/onRead remain on the host by default; optional read offload uses a shared worker.
  • onDisconnect always runs on that worker, after deferred reads.
  • Blocking writes belong on application workers. Owners stop acceptance, disconnect, wait for isIdle and drain their own workers before deleting a channel; no deletion from its callback.
  • Channel construction/destruction belongs to one application management task.

Validation:

  • 16 native concurrency/error/ownership/pool/lifecycle tests pass, also with AddressSanitizer/UBSan and a CMake Release build.
  • Actual CoC-enabled compile/link probes pass on ESP32-S3 with IDF 5.4.3 and 5.5.1, both inline/deferred modes; an IDF 6.1 build also passes.
  • Add CI for those stable-IDF CoC configurations and a bounded worker-driven echo probe. These are compile/link checks, not RF tests.

The host-task policy overlaps conceptually with #419 and should be coordinated during review. HCI resource windows, private header diagnostics, MPS alignment and the standalone measurement harness are outside this PR. Earlier hardware measurements informed the design; they are not presented as RF acceptance of every extracted commit.

The native host-event test shim now enqueues outside assert(), so disabling assertions does not disable its scheduling. This fixture change is separate from the production lifecycle behavior.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • 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.

This branch has not been deployed

No deployments
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.

1 participant