Repository navigation
feat(ffi): own C++ handles through smart pointers, shared where clones match - #430
Merged
Merged
Conversation
…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>
…ng count Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
marked this pull request as ready for review
November 2, 2026 16:18
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
kixelated
enabled auto-merge (squash)
November 2, 2026 16:23
|
codex / Codex land fixer note:
(written by codex / Codex) |
This was referenced Nov 2, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 everyClonetype's copy crossed into Rust for a new value, even where one shared value could not be told apart from a clone.Approach
rt.hppgainsdetail::unique, overstd::unique_ptrwith a deleter calling<obj>_free, anddetail::shared, overstd::shared_ptr. A generated class holds one and follows the rule of zero. A uniqueClonetype is move-only with an explicitclone()that crosses into Rust; a shared type copies by sharing the handle, and the last copy frees it on whichever thread drops it.collect.rs,Object::shared). A type shares only if it isCloneandSync, no export borrows it mutably (&mut self, a&mutparameter, or a&mutlent to a closure), and its clone owns no state that changes behind&. The last check walks the type's fields through rustc: anything behindArc,Rc, theirWeaks, or&Tis shared; every other field has to clone with a derivedCloneor std's containerClone, have noDrop, and hold only such values. SoBox<OnceLock<_>>, which isFreezebut clones independently, keeps copies apart, and so does any hand-writtenCloneorDrop, which may count clones. std'sCloneof aCopytype, such asNonZero's, is trusted to copy.Syncis required because a non-Synctype's&selfborrows are held on one thread at a time, and two threads' copies would collide on that.selfmethods stay&&and by-value handles still move in, throughaccess::own. On a shared copy that another copy still holds,ownmoves a clone out instead, which leaves the other copy whole, as Rust does.shared_ptr, and keeping a copy of it past the call aborts.viewandbufwere already contiguous ranges, so under C++20 they convert tostd::span<const std::uint8_t>and back. That is now documented and tested. The header stays C++17.lifetimesmode intests/api.cpp, with new fixture typesGauge(shared through anArc, with aDropcounter) andLatch(aBox<OnceLock>). They cover:clone()keeping values apart;bufoutliving its object and owning Rust's buffer;a handle is null) and a lent object copied past its call.view, span, or lentviewread after its bytes are gone.moq's export shares 66 types. Among them are
path,timestamp,url,timescale, the statsregistry/handle,cache_pool, andquic_driver_send_group. 152 cross, including every producer and consumer, because kio's hand-writtenCloneandDropcount them.Impact
Clonetype whose copies may not share is move-only withclone().T copy = t;becomest.clone(), and a braced list of such objects becomespush_backs. Such a type with a Rust method namedcloneneeds arename.Arc's clones, such asArc::strong_count, can observe the difference.ir::Objectgainsshared(crate-internal).ffi/MIGRATE.mdhas the entry.ffi/moq/moq.cpp), which now sharespathand clonesbroadcast_producerandtokio_clientexplicitly.ffi/moq/bench/export.cppbuilds unchanged.export.cppat-O2, minimum of 6 alternating runs against the same library: stamped frame 254 to 265 ns, which is the sharedtimestamp's control block per handle. Reused timestamp, handle call, and write+read are unchanged within noise.Alternatives
Freezeand no&mut self) wrongly sharesBox<Mutex/OnceLock>-style types, so I rejected it.&mutcalls on shared types would share more types, but it breaks lent&mutobjects and departs from the plan.selfor by-value uses from sharing is simpler, but it would leaveurland others crossing for no observable gain.own()keeps them shared.Cloneimpls that only clone fields would share moq's producers, but proving that needs MIR analysis, so the guard stays conservative.Follow-ups
&selfcall on a shared copy is still pending after that copy is destroyed, a move out of the last remaining copy aborts witha handle is borrowedinstead of cloning, because Rust's handle is borrowed by the future and invisible touse_count.quest/m1/ffi-export/cpp.mdstill tracks MSVC.Decision
The maintainer chose option A on 2026-11-02:
Arc- andRc-backed types share throughstd::shared_ptr. Every C++ copy of a shared object is one Rust value, soArc::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, thesharedhandle's comment inruntime/rt.hpp, the generator's docs, andMIGRATE.mdstate this. The alternatives were sharing only types that reach noArcorRc(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.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 asquest/m3/ffi-cpp-shared-future.mdin moq-dev/rust2#433, and it fails loud through Rust's borrow check, never undefined behavior.collect.rs: anArc's clone count observes sharing. Settled by the maintainer's option A (see Decision); documented.cpp.rs:clone()in a class namedclonedid not compile. Fixed in 67778e8 byapart, which also covers the same pre-existing clash forto_string,debug, anddefault_.just cipassed on b9fef4e, which merges origin/main through #423 and #419.🤖 Generated with Claude Code