Skip to content

feat(ffi): own C++ handles through smart pointers, shared where clones match - #430

Merged
moq-bot[bot] merged 7 commits into
mainfrom
lifetimes/cpp
Nov 2, 2026
Merged

moq-bot[bot] merged 7 commits into
mainfrom
lifetimes/cpp

Conversation

@kixelated

@kixelated kixelated commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The C++ item of quest/m1/ffi-export/lifetimes.md, decided with the maintainer on 2026-11-01: C++ objects owned a raw handle with hand-written move and copy members, and every Clone type's copy crossed into Rust for a new value, even where one shared value could not be told apart from a clone.

Approach

  • Handles. rt.hpp gains detail::unique, over std::unique_ptr with a deleter calling <obj>_free, and detail::shared, over std::shared_ptr. A generated class holds one and follows the rule of zero. A unique Clone type is move-only with an explicit clone() that crosses into Rust; a shared type copies by sharing the handle, and the last copy frees it on whichever thread drops it.
  • Clone guard, at compile time (collect.rs, Object::shared). A type shares only if it is Clone and Sync, no export borrows it mutably (&mut self, a &mut parameter, or a &mut lent to a closure), and its clone owns no state that changes behind &. The last check walks the type's fields through rustc: anything behind Arc, Rc, their Weaks, or &T is shared; every other field has to clone with a derived Clone or std's container Clone, have no Drop, and hold only such values. So Box<OnceLock<_>>, which is Freeze but clones independently, keeps copies apart, and so does any hand-written Clone or Drop, which may count clones. std's Clone of a Copy type, such as NonZero's, is trusted to copy. Sync is required because a non-Sync type's &self borrows are held on one thread at a time, and two threads' copies would collide on that.
  • Moves keep their semantics. self methods stay && and by-value handles still move in, through access::own. On a shared copy that another copy still holds, own moves a clone out instead, which leaves the other copy whole, as Rust does.
  • Lent objects are still references for one call. A lent shared object uses a non-owning shared_ptr, and keeping a copy of it past the call aborts.
  • Spans. view and buf were already contiguous ranges, so under C++20 they convert to std::span<const std::uint8_t> and back. That is now documented and tested. The header stays C++17.
  • Tests are a new lifetimes mode in tests/api.cpp, with new fixture types Gauge (shared through an Arc, with a Drop counter) and Latch (a Box<OnceLock>). They cover:
    • copies sharing writes;
    • moving out of a shared copy, and of the last copy;
    • clone() keeping values apart;
    • a copy released on one thread while another copy's call runs;
    • a lent copy used during its call;
    • buf outliving its object and owning Rust's buffer;
    • span to view and view to span under C++20.
  • New abort cases: a moved-from object (a handle is null) and a lent object copied past its call.
  • C++'s own undefined behavior, documented: a dangling reference, and a view, span, or lent view read after its bytes are gone.

moq's export shares 66 types. Among them are path, timestamp, url, timescale, the stats registry/handle, cache_pool, and quic_driver_send_group. 152 cross, including every producer and consumer, because kio's hand-written Clone and Drop count them.

Impact

  • Breaking, C++: a Clone type whose copies may not share is move-only with clone(). T copy = t; becomes t.clone(), and a braced list of such objects becomes push_backs. Such a type with a Rust method named clone needs a rename.
  • Breaking, C++: a shared type's copy shares the handle instead of cloning in Rust. Only a count of an Arc's clones, such as Arc::strong_count, can observe the difference.
  • Breaking, C++: keeping a copy of a lent shared object past its call aborts.
  • ir::Object gains shared (crate-internal).
  • ffi/MIGRATE.md has the entry.
  • Consumer: moq's C++ (ffi/moq/moq.cpp), which now shares path and clones broadcast_producer and tokio_client explicitly. ffi/moq/bench/export.cpp builds unchanged.
  • Benchmark, export.cpp at -O2, minimum of 6 alternating runs against the same library: stamped frame 254 to 265 ns, which is the shared timestamp's control block per handle. Reused timestamp, handle call, and write+read are unchanged within noise.

Alternatives

  • A naive guard (Freeze and no &mut self) wrongly shares Box<Mutex/OnceLock>-style types, so I rejected it.
  • Copy-on-write for &mut calls on shared types would share more types, but it breaks lent &mut objects and departs from the plan.
  • Excluding types with self or by-value uses from sharing is simpler, but it would leave url and others crossing for no observable gain. own() keeps them shared.
  • Trusting hand-written Clone impls that only clone fields would share moq's producers, but proving that needs MIR analysis, so the guard stays conservative.

Follow-ups

  • This PR conflicts with moq-dev/rust2#427, which rewrote the same quest. Only the C++ bullet and the Public API mention are removed here. Keep feat(ffi): drop close() from Swift's and Python's objects #427's text and drop the C++ bullet when merging.
  • Known gap: if a future from a &self call on a shared copy is still pending after that copy is destroyed, a move out of the last remaining copy aborts with a handle is borrowed instead of cloning, because Rust's handle is borrowed by the future and invisible to use_count.
  • MSVC was not run (no MSVC here). The new code is portable C++17. quest/m1/ffi-export/cpp.md still tracks MSVC.

Decision

The maintainer chose option A on 2026-11-02: Arc- and Rc-backed types share through std::shared_ptr. Every C++ copy of a shared object is one Rust value, so Arc::strong_count, Rc::strong_count, or a weak count of what it holds counts one for all its copies, where Rust's clones would count one each. ffi/README.md's C++ "Sharing" bullet, the shared handle's comment in runtime/rt.hpp, the generator's docs, and MIGRATE.md state this. The alternatives were sharing only types that reach no Arc or Rc (B), or adding a whole-program scan for count reads (C).

Review

mechanical fix by codex / Codex: Merged base and re·solved documentation conflicts preserving both sides; passed cargo build --locked --manifest-path ffi/Cargo.toml; cargo test --locked --manifest-path ffi/Cargo.toml (4 unit and 19 integration tests, 1 Android test ignored); cargo fmt --manifest-path ffi/Cargo.toml --all --check; cargo clippy --locked --manifest-path ffi/Cargo.toml --all-targets -- -D warnings; used installed pinned nightly with KOTLINX_COROUTINES and Swift library paths set. (head 8ca2e22)

Round 4 of quest review --base origin/main: Codex, gpt-6.1-sol, medium effort, head b9fef4e. No findings.

  • Round 3, rt.hpp: a pending future from a destroyed shared copy makes a move out of the last copy abort with "a handle is borrowed" instead of cloning. Refuted as out of scope: it is the known gap under Follow-ups, planned as quest/m3/ffi-cpp-shared-future.md in moq-dev/rust2#433, and it fails loud through Rust's borrow check, never undefined behavior.
  • Round 2, collect.rs: an Arc's clone count observes sharing. Settled by the maintainer's option A (see Decision); documented.
  • Round 1, cpp.rs: clone() in a class named clone did not compile. Fixed in 67778e8 by apart, which also covers the same pre-existing clash for to_string, debug, and default_.

just ci passed on b9fef4e, which merges origin/main through #423 and #419.

🤖 Generated with Claude Code

kixelated and others added 3 commits November 2, 2026 02:39
…s match

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

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 304d2036-6fdc-43c0-93bc-99cfde455725
  • 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

@kixelated
kixelated enabled auto-merge (squash) November 2, 2026 16:23
@moq-bot
moq-bot Bot disabled auto-merge November 2, 2026 17:07
@moq-bot
moq-bot Bot enabled auto-merge (squash) November 2, 2026 17:07
@moq-bot

moq-bot Bot commented Nov 2, 2026

Copy link
Copy Markdown

codex / Codex land fixer note:

Merged base to resolve the two documentation conflicts. Kept both C++ and Swift/Python migration entries, retained the new lifetime item from base, and combined the completed-plan removals so the remaining public API plan covers Kotlin bytes. No behavior changed. FFI build, formatting, Clippy, and all enabled unit and integration tests passed with the installed pinned nightly and required Kotlin/Swift environment paths.

(written by codex / Codex)

@moq-bot
moq-bot Bot merged commit 15ccb34 into main Nov 2, 2026
@moq-bot
moq-bot Bot deleted the lifetimes/cpp branch November 2, 2026 17:48
moq-bot Bot pushed a commit that referenced this pull request Nov 2, 2026
Every item it lists has landed: Go's Close and bytes (#411), Swift's and Python's close
(#427), Kotlin's bytes (#432), C++'s smart pointers (#430), and the lend's end (#429).
C stays out of scope. Its dependents, Kotlin's native memory and C++'s shared move-out,
are no longer blocked.

Co-authored-by: Luke Curley <432854+kixelated@users.noreply.github.com>
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant