Soundness review request: SRCE 0.1

This crate is supposed to let GhostCell/LCell work with tokio

It works by masking the token's lifetime and pretending it's 'static. But we have no idea if this is sound.

Obviously, it can't be .awaited on, so it needs to be reopened across .await points. But it's not too bad since it's pretty trivial to get data in and out of the LCellEnvironment. That means it only requires minor restructuring around .await instead of the major restructuring required to avoid LCell entirely.

But that's assuming it's even sound.

1 Like

Some small example code that uses the API as intended would be useful to get started :slight_smile:

yes, that is the fn test at the end of the src/

(we're too lazy and sleep deprived to make it part of the docs proper, at this time)

1 Like

No worries, that’s completely sufficient for now; just hard to discover, but that’s irrelevant for reviewing. Thanks for the pointer

1 Like

So, the API looks reasonably sound to me upon first inspection.


Yeah, we figured the basic idea ("closing" and "reopening" an LCellOwner) is sound, it's a perfectly fine use of UnsafeCell when you dig into it. (it may be a different LCellOwner for the same LCells, but only one of them exists at a time (mutably) or they're only shared, so it doesn't really matter that it's a different LCellOwner because it can't cause unsoundness. if we're making sense here. we don't think we know how to explain this to other ppl actually.)

The real question is whether the lifetime sharing between the LCellOwner 'id and the selfref holder is sound. We don't know. We haven't been able to work out a solution to that one.

Also, we suppose generativity can be used, directly, to make LCellOwner work with tokio. Since generativity turns an outer function into a scope, it can even turn an async fn into a scope, right? Ah well, so our motivation for actually publishing this wasn't really useful after all. But it can still do weird, niche stuff, that generativity by itself cannot.

That was a point of interest for me two. Initially it seems like mixing two independent concerns. On the other hand, the key point for soundness of a LCellOwner is that its lifetime is unique, and e.g. the Holder API should ensure that lifetime 'x passed to the F: for<'x> FnOnce(OperateIn<'x, T>) -> R, from different invocations are always distinguished. It’s necessary for the soundness of Holder itself, because the Holder doesn’t statically know its own lifetime; the “real” lifetime for 'x would be something like “until Holder is dropped” which is a dynamic concept, so static typing will always approximate/generalize this, and work with universally quantified lifetimes to stay sound.

One notable exception though in the following context of possible future API changes: I think I’ve suggested that previously, and also I haven’t actually reviewed whether I’m certain that this would be actually sound, but there’s this potential improvement of the selfref::Holder API. The current API

pub fn operate_in<'i, F, R>(self: Pin<&'i Self>, f: F) -> R
    F: for<'x> FnOnce(OperateIn<'x, T>) -> R,

does not convey to F the information that self is borrowed at least for 'i, and thus it would be safe for F to assume that 'x: 'i. This can be useful because it would allow R to borrow from self, at least if Pin<&'x T::Kind<'x>> allows you to extract some Foo<'x> data that’s covariant in 'x, so it can be converted to Foo<'i> and returned from the closure.

One way to convey this bound to the closure would be to add a &'i &'x () phantom argument; or perhaps to add 'i as a new extra lifetime argument to OperateIn, and give it a 'x: 'i on the type definition of OperateIn.

Now, if this API change is sound for selfref, the same thing would still not be true for an analogous change to srce, because of the additional requirement of the lcell owner to be unique. With the API

pub fn open_rw<'i, F, R>(self: Pin<&'i mut Self>, f: F) -> R
    F: for<'a, 'id> FnOnce(&'a mut LCellOwner<'id>, OperateIn<'id, T>) -> R,

if: the closure F got the additional information that 'id: 'i, then this information has the potential to nail down the 'id lifetime in one specific case so it’s no longer a “unique” lifetime: if 'i was 'static! And of course, by leaking the LCellEnvironment, a Pin<&'static Self> can be constructed; the closure, if provided the additional 'id: 'i (so in that case 'id: 'static) bound, would then have access to a &'a mut LCellOwner<'static>, the process could be repeated twice in a nested manner, and ultimately there would be two usable &mut LCellOwner<'static> values co-existing.

I don’t immediately see the same kind of issue with the analogue change to open_ro though, because even a &LCellOwner<'static> seems unproblematic, since it’s a shared reference only.

Haven’t tested it, but that sounds like it might work, good observation!

1 Like

I had not thought about this benefit of the macro method of generating brand lifetimes, or if I had, it's been so long that I've forgotten.

But yes, make_guard! functions to generate a fresh lifetime brand even in an async context, without requiring introducing a sync closure scope.

A key part of the soundness reasoning here is that while generativity::Guard makes the extremely strong promise that its 'id is unique and nonunifiable with any other (constrained[1]) invariant "branded" lifetime — this is necessary for LCellOwner or any other consumer to accept it as a fresh brand in safe API — the requirement on LCellOwner or essentially any other consumer of lifetime brands is significantly looser: just that the 'id brand is unique among LCellOwner, to prevent using multiple LCellOwner with the same LCell.

While getting &LCellOwner<'static> is probably sound, since this only allows shared referencing, I would personally recommend avoiding it, because it really strains the logic of LCell which is based on the brand being unique.

  1. Specifically, the uniqueness proof relies on the lifetime region's end point being uniquely constrained. Doing so sufficiently enough either requires the properly universal for<'a> to generate an unknown lifetime, or the extremely careful and tight bound that make_guard! manufactures in the function scope's drop glue. ↩︎

1 Like

we wonder if we should just bring this API into selfref itself then... is there any reason why we might not want to?

tho at the same time the clean separation of documentation is nice... but we can just use a module we guess.

@steffahn is this sound under Pin::set, also? we kinda forgot about that one... (tho we can't think of any way it wouldn't be...)

I don’t see a problem with it either.

1 Like