Skip to content

fix(ffi): move a clone out of C++'s last shared copy while a future borrows it - #451

Merged
moq-bot[bot] merged 6 commits into
mainfrom
quest/m3/ffi-cpp-shared-future
Nov 2, 2026
Merged

moq-bot[bot] merged 6 commits into
mainfrom
quest/m3/ffi-cpp-shared-future

Conversation

@kixelated

@kixelated kixelated commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

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, by use_count. A future made by a &self call borrows the value in Rust (borrow_arc) without holding a C++ copy. So once the copy that made the future is destroyed, use_count reads 1, the last copy moves out the borrowed value, and Rust aborts with a 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.

  • New <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 of state != 0). The C name is claimed like _free, so a method named borrowed on such a type takes a rename.
  • Sound against races: with use_count at 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.
  • A moved-from copy (null handle) skips the query, and a moved-out value reads as not borrowed. Either way the call itself refuses it and names itself, as before.

Tests (tests/api.cpp, run by api_cpp at 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 with api_gauge_into_level: a handle is borrowed.
  • broken cases shared moved from and shared moved out: moving out of a moved-from shared copy, or out of one already moved out, aborts naming api_gauge_into_level. Without the null check the first names api_gauge_borrowed, and without the moved-out exclusion the second names api_gauge_clone.

The fixture gains Gauge::later, an async fn(&self) that reuses yielded.

Impact

  • C header: new <object>_borrowed for each object whose C++ copies share, such as moq's path, timestamp, and url. The contract changes with it.
  • C++: no API change. detail::shared takes a fourth template parameter.
  • Breaking only for a type that shares and has a method named borrowed, which now takes a rename (ffi/MIGRATE.md).
  • Consumers: moq's C++ copies of path, timestamp, and url, which can be moved out of while a future made from one pends. just ffi moq passes.

Alternatives

  • The future holds a shared_ptr copy of each shared handle it borrows, so use_count sees it. No new C function, but the copies have to be carried through future, then's driver, and via's awaiter, 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 an Arc's count. It also needs an acquire fence after use_count's relaxed load before the move reads Rust's state. Asking Rust covers every borrow from one place.

Follow-ups

None.

Checks

  • just ci and just ffi moq pass locally on this head.

Review

Codex (quest review), gpt-6.1-sol, medium effort, round 4, head beef19f: no findings.

  • Round 1 (f5cc9cf): no findings. My own review then found that a moved-from copy aborted naming _borrowed: fixed in 87e0949.
  • Round 2 (87e0949), low: a moved-out (TAKEN) handle read as borrowed, so a second move aborted naming _clone. Fixed in 941370c, with a test.
  • Rounds 3 (941370c) and 4 (beef19f, a rustfmt wrap only): no findings.

🤖 Generated with Claude Code

kixelated and others added 5 commits November 2, 2026 12:03
…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 kixelated changed the title C++: a move out of a shared copy while its future pends fix(ffi): move a clone out of C++'s last shared copy while a future borrows it Nov 2, 2026
@kixelated
kixelated marked this pull request as ready for review November 2, 2026 20:25
@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: 690c8d33-03ed-4dc5-8cf4-4e0318170b33
  • 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 20:57
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