Repository navigation
fix(ffi): move a clone out of C++'s last shared copy while a future borrows it - #451
Merged
Merged
Conversation
Quest-Claim: 09c0a5b59fe79fe0
…orrows it 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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
marked this pull request as ready for review
November 2, 2026 20:25
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
kixelated
enabled auto-merge (squash)
November 2, 2026 20:57
This was referenced Nov 2, 2026
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
In the generated C++, a type whose copies share one handle (through
std::shared_ptr, from moq-dev/rust2#430) moves out a clone when another copy holds the handle, byuse_count. A future made by a&selfcall borrows the value in Rust (borrow_arc) without holding a C++ copy. So once the copy that made the future is destroyed,use_countreads 1, the last copy moves out the borrowed value, and Rust aborts witha handle is borrowed. In Rust, the clone-then-move would succeed.Approach
The move asks Rust:
shared::own()moves out a clone when another copy shares the handle or Rust says the value is borrowed.<object>_borrowed(const <object> *)in the C header, only for objects whose C++ copies share. It reads the handle's borrow state (rt::Object::borrowed, an acquire load ofstate != 0). The C name is claimed like_free, so a method namedborrowedon such a type takes arename.use_countat 1, no other C++ copy can start a borrow, and a future never borrows again after it is made. So "not borrowed" stays true until the move, and "borrowed" at worst clones a value whose future just finished, which no call can tell apart.Tests (
tests/api.cpp, run byapi_cppat each standard):lifetimes: a gauge copy makes a future, which pends after one poll; that copy is destroyed; the last copy moves out (a clone), and the future then reads the value and frees it once it finishes. Without the fix this aborts withapi_gauge_into_level: a handle is borrowed.shared moved fromandshared moved out: moving out of a moved-from shared copy, or out of one already moved out, aborts namingapi_gauge_into_level. Without the null check the first namesapi_gauge_borrowed, and without the moved-out exclusion the second namesapi_gauge_clone.The fixture gains
Gauge::later, anasync fn(&self)that reusesyielded.Impact
<object>_borrowedfor each object whose C++ copies share, such as moq'spath,timestamp, andurl. The contract changes with it.detail::sharedtakes a fourth template parameter.borrowed, which now takes arename(ffi/MIGRATE.md).path,timestamp, andurl, which can be moved out of while a future made from one pends.just ffi moqpasses.Alternatives
shared_ptrcopy of each shared handle it borrows, souse_countsees it. No new C function, but the copies have to be carried throughfuture,then'sdriver, andvia'sawaiter, for the receiver and every&object argument. Each must be released when the Rust future is freed, not when its task is destroyed, or a needless clone shows in anArc's count. It also needs an acquire fence afteruse_count's relaxed load before the move reads Rust's state. Asking Rust covers every borrow from one place.Follow-ups
None.
Checks
just ciandjust ffi moqpass locally on this head.Review
Codex (
quest review), gpt-6.1-sol, medium effort, round 4, head beef19f: no findings._borrowed: fixed in 87e0949.TAKEN) handle read as borrowed, so a second move aborted naming_clone. Fixed in 941370c, with a test.🤖 Generated with Claude Code