Skip to content

fix(net): prove native futures Send at the default recursion limit - #5682

Merged
kixelated merged 9 commits into
mainfrom
quest/m2/send-depth
Oct 16, 2026
Merged

kixelated merged 9 commits into
mainfrom
quest/m2/send-depth

Conversation

@kixelated

@kixelated kixelated commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Native futures awaiting announcements or track/group reads overflow nightly's default-depth Send proof. Raising moq-net's recursion limit does not raise a downstream crate's limit.

Approach

Shorten the equivalent Send/Sync proofs at kio locks, check native shared-origin thread safety at construction, and expose shallow opaque proofs for nested private futures. Remove the raised recursion limit. Add a default-limit integration crate and a nightly CI gate that denies recursion-depth warnings.

Impact

  • Public API and MoQ wire format are unchanged. Existing native trait bounds and wasm's non-Send values are preserved.
  • No additional runtime allocation.

Validation

Reproduced the original overflow on pinned nightly 2026-10-04; the proof passes at the default limit after merging current main. Removing the checked origin wrapper reproduces overflow. The compile_fail doctests for SendCell and kio::Lock now fail on E0277 rather than a parse error. just check and CI (including io_uring) pass.

Alternatives

Conditional lock proofs alone remain too deep. Additional boxing would allocate; shallow checked proofs avoid that cost. Private future signature experiments were removed.

Follow-ups

None.

Closes #4648.

(Written by GPT-6, updated by Opus 5.5)

kixelated and others added 6 commits October 15, 2026 20:23
Co-Authored-By: GPT-6 <noreply@openai.com>
Add a default-recursion-limit integration crate proving announcement, track, and group futures are Send. Baseline compile is still running; no production change has been made. A nightly toolchain is absent from the local environment, and the explicit nightly CI proof remains to wire. No API or wire changes.

Co-Authored-By: GPT-6 <noreply@openai.com>
Nightly default-limit failure reproduced. Conditional lock bounds and constructor-checked origin state added; opaque nested future proof under validation. Stable baseline passed. Full checks and final nightly confirmation remain.

Co-Authored-By: GPT-6 <noreply@openai.com>
Pinned nightly default128 compile passed with the origin wrapper and private future boundaries. Removing the wrapper reproduced overflow, so retain it. Full CI lint/feature and native non-ring suites are running; loom and doctests remain.

Co-Authored-By: GPT-6 <noreply@openai.com>
Final nightly default-limit compile passed. Native and CI feature/lint suites are running, doctests queued. No public API or wire change.

Co-Authored-By: GPT-6 <noreply@openai.com>
Pinned nightly default128 proof,7749 non-ring workspace tests,466capture,192play,30loommodels,doctests and strict CIcheck passed. Full io_uring runtime fixture remains unavailable under previously reproduced setup ENOMEM; it was not retried.

Co-Authored-By: GPT-6 <noreply@openai.com>
kixelated and others added 2 commits October 15, 2026 23:22
In statement position, include! of an item file does not parse, so both
doctests passed regardless of SendCell's constructor bound. Include at item
level and pin the expected E0277 on both SendCell and kio Lock.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review October 16, 2026 06:50
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 39 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5d0052c6-9bc7-4c38-bf6b-e25e1d57b1dd
📥 Commits

Reviewing files that changed from the base of the PR and between a0646e1 and d212770.

📒 Files selected for processing: 13
  • .github/workflows/nightly.yml
  • quest/m2/README.md
  • quest/m2/send-depth.md
  • rs/justfile
  • rs/kio/src/lock.rs
  • rs/moq-net/src/auth.rs
  • rs/moq-net/src/ietf/subscriber.rs
  • rs/moq-net/src/lib.rs
  • rs/moq-net/src/model/origin.rs
  • rs/moq-net/src/util.rs
  • rs/moq-net/src/util/cell.rs
  • rs/moq-net/tests/send.rs
  • sh/rs/send-depth.sh
  • 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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-16T17:40:45.893740Z d212770 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Merge summary:

  • Merged current main (clean, no conflicts) to pick up fix(moq-mux): judge a source's clock on slack the floors saw #5714. The previous TS validation failure was the full-multiplex clip exiting on "+25990.1 ppm off ours", the positive-drift startup misread fix(moq-mux): judge a source's clock on slack the floors saw #5714 fixes. This PR touches no TS or moq-mux code. TS validation now passes on d212770, with every other check green.
  • Ran the new nightly gate's check by hand on the merged tree with nightly 2026-10-04 from the flake's rust-overlay: cargo check -p moq-net --test send with -D recursion_depth_exceeding_limit passes, and the lint name is recognized. Reading nightly.yml: the include adds a separate rs send-depth row, only that row installs nightly through rustup, and sh/rs/send-depth.sh picks rustup's sysroot because nix's stable rustc comes first in PATH. The CI env does not deny warnings, so nightly-only deprecations (fetch_update) will not fail it. I did not dispatch the nightly workflow on this branch.
  • Rechecked the unsafe impls: kio::Lock's T: Send bounds match Arc<Mutex<T>>'s derived ones, and SendCell is only constructible through its T: Send + Sync constructor. RequestGrant::guard stays lazy, since until is itself an async fn.
  • Reviews: Codex finished with no findings on d1cbbd9. CodeRabbit hit its rate limit. The only change since then is the clean merge of main.
  • just check passes locally.

(Written by Claude Opus 5.5)

@kixelated
kixelated added this pull request to the merge queue Oct 16, 2026
Merged via the queue into main with commit ebce2e1 Oct 16, 2026
13 checks passed
@kixelated
kixelated deleted the quest/m2/send-depth branch October 16, 2026 18:41
kixelated added a commit that referenced this pull request Oct 20, 2026
Nightly's send-depth gate and every fuzz target have failed since #5773
gave a stats session and its wire counters references to each other: each
IETF subscriber future repeated that recursive auto-trait proof and
overflowed the default recursion limit. Bisected to that commit with
nightly 2026-10-19.

Wrap each shared session context in SendCell, as #5682 did for the
origin graph, so the proof runs once at construction. No allocation or
API change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant