Soundness of pac_cell library

code review
1

I've created a PacCell (parent and child) for a specific use case that I described in this topic.

Can I get an early code review / soundness check?

Library code (fits on one screen, almost): pac_cell/src/lib.rs at main · aljazerzen/pac_cell · GitHub
A real-world example: pac_cell/tests/it/main.rs at main · aljazerzen/pac_cell · GitHub

I would be really thankful to any Rust gurus out there.

2

new is unsound, as it lets the user choose the lifetime 'p and then exposes a reference with lifetime 'p without any restriction on it. The user can thus choose 'static and get a &'static mut that really only lives for the lifetime of the PacCell.

let mut s = None;
drop(PacCell::new(String::from("foo"), |p| s = Some(p)));
println!("{s:?}"); // use after free, prints random stuff

Edit: in general you cannot store the child type C with its lifetime, as the actual lifetime is not expressible to the compiler. This is why all self-referential crates either use a macro and internally erase it (self-cell and ouroboros), use some weird trait that generalizes the type over its lifetime (e.g. Yokable in yoke) or are limited to store only a specific kind of type (e.g. the child can only be a reference to some other type). You should consider using one of those approaches otherwise there will likely continue to be issues around your child lifetime.

2 Likes
3

So what you are saying is that this change is needed:

     pub fn new<F>(parent: P, child_constructor: F) -> Self
     where
-        F: FnOnce(&'p mut P) -> C,
+        F: for<'a> FnOnce(&'a mut P) -> C,

... but that such new would construct child with a lifetime param that is correct, but cannot be expressed when storing it into the backing struct.

Looking at yoke and self_cell, they so something very similar, but also expose the parent (or cart) which means that child (or yoke) can only borrow cart immutably. This makes me wonder if a sound API even exists for what I'm trying to do here.

4

Kinda. You do need an existential lifetime (the for<'a> ...) but as you see just doing this is not gonna work because C cannot name that lifetime. In fact referencing C is wrong, a PacCell<Parent, Child<'???>> has no correct lifetime to put in the '??? as it is not borrowing anything external, it is a self-borrow.

Ideally you would be able to say that C is some type generic over 'a, i.e. C<'a>, and then your bound would become for<'a> FnOnce(&'a mut P) -> C<'a>, but unfortunately this syntax is not possible. However you can kinda imitate this with a trait, which is what yoke::Yokable does.

Macro-based crates instead just don't provide a generic API that depends on C as they can just manipulate the AST to fill the lifetime with 'static when storing C internally (of course without ever exposing it to safe code).

I don't see a reason why such an API cannot be sound, though it is surely not compatible with exposing the parent/cart like yoke/self_cell do. So if you don't expose the parent/cart it should be fine.

2 Likes
5

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.