Add support for FD passing - #666
Draft
emilazy wants to merge 13 commits into
Draft
Conversation
emilazy
force-pushed
the
push-xstmntksusmk
branch
from
June 10, 2026 16:46
28e5629 to
cfbcb9b
Compare
Switch to a deterministically‐ordered `BTreeSet` to avoid nondeterministic type parameter ordering, observable when generating code for the `Persistent` interface in `/capnp/persistent.capnp` in the `persistent::SaveResults` type. I am not sure whether `BTreeSet` produces the ideal ordering in all cases, but it at least works as expected for that interface, and a nondeterministic ordering is clearly no good. (Generating code for `/capnp/persistent.capnp` runs into another issue due to a module name collision between the `persistent` annotation and the `Persistent` interface, but that’s out of scope here; the bug can be reproduced in the generated source files regardless, and commenting out the annotation definition makes the generated code compile.)
These are unnecessary; the only mutable state within `PipelineClient` is already behind its own `RefCell`.
We only ever mutate `remote_ref_count`; as a side benefit, we can use a plain `Cell`, since it’s a `u32`, we only borrow it mutably for a simple addition, and we only read it in the `Drop` implementation, where there are definitionally no other references that could borrow it mutably.
emilazy
force-pushed
the
push-xstmntksusmk
branch
from
July 20, 2026 22:29
cfbcb9b to
627a893
Compare
emilazy
force-pushed
the
push-xstmntksusmk
branch
from
July 20, 2026 22:30
627a893 to
cb619b2
Compare
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
1 participant
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.
I needed this and it didn’t seem too complicated to implement based on capnproto/capnproto#821; famous last words :)
This is largely based on that PR (and it’s possible I’ve missed fixes postdating it), but with some divergences for Rust’s ownership model and existing differences in the Rust library. In particular, since we need a bespoke IO abstraction to handle FD passing anyway (sadly, all the existing options in the ecosystem are somewhat lacking), I took the opportunity to make the
futures-iocoupling optional and allow more direct use of Tokio.I’m marking this as a draft for now, because there’s a fair number of unresolved questions and missing pieces, and I’m fairly likely to make changes:
It’s not entirely clear how to get the best ergonomics here; you usually want an
OwnedFdon the receiving end, which currently requires an explicit clone, but avoiding that brings up a lot of questions around mutability and what the server‐side interface should be like. A lot of theRefCell‐reducing churn here is to make usingBorrowedFdwork, which is nice on the server end but sort of annoying on the client end. The current interface matches the C++ implementation pretty closely, although KJ is a bit more cavalier around FD ownership than Rust is.The IO interfaces don’t feel fully baked yet; in particular, I think it may be a more natural fit to the IO patterns to use an ownership‐transferring buffer model, which would also potentially work better with completion‐based interfaces like
io_uring.There ought to be an implementation of buffering that handles FDs, as the C++ version has.
There are a good few contortions here to try and achieve perfect SemVer compatibility and keep the existing
futures-io‐based interfaces working by default. If it’s acceptable to make breaking changes, a good portion of this could be simplified.However, it should be fairly complete and ready for feedback on the basic approach. There’s a fair bit of entangled stuff and some side quests in here; I strongly recommend reading this commit‐by‐commit. Some of it could be split out, and some of it could potentially be dropped or simplified based on the direction this takes, but I’ve been iterating on this locally for long enough and want to get a draft up before it diverges even further :)
Closes: #625