Footgun with Rc::assume_init and related methods

[Diggsey]

Location

I came across https://doc.rust-lang.org/nightly/std/rc/struct.Rc.html#method.assume_init recently, and spotted a footgun that I think ought to be called out:

Summary

The safety section does not clarify whether multiple Rcs are allowed to exist when assume_init is called. If they are, then whether Drop is called on the inner value will depend on the drop order of those Rcs. In the case of Arc this might well be non-deterministic.

IMO, the documentation should specify whether this is allowed, and if so should point out that callers must take care around this potential issue.

[lukaslueg]

If we Rc::<MaybeUninit<T>>::assume_init() one instance, other instances remain in the Rc<MaybeUninit<T>> state, and those will never call drop on T, unless those instances assume initialization as well. Getting this right - if one instance can assume initialization, everyone has to - is left as an exercise to the reader. FWIW I fail to see how this could lead to memory unsafety. But it should be fairly easy to leak memory/resources that way

• Initialize the inner T where T: Drop
• At least one instance of Rc<MaybeUninit<T>> remains uninitialized even when T is in fact initialized
• The above instance will prevent other Rc<T> from dropping the inner T when they go out of scope
• The remaining Rc<MaybeUninit<T>> does not drop T because MaybeUninit<T> never does.
• T remains alive forever and is unreachable

[Diggsey]

FWIW I fail to see how this could lead to memory unsafety.

eg. Pin requires that the destructor is called before the memory is reused, but Rc<MaybeUninit<T>> will free the memory without calling the destructor.

[hanna-kruppe]

That's a good point. Luckily, there are some factors that work against this happening by accident:

1. The natural and recommended way to do initialization goes through &mut MaybeUninit<T> to get a *mut T to write through. Getting &mut T from Rc<T> or Arc<T> requires exclusive access, i.e., refcount == 1, so there can't be other references to the same MaybeUninit<T> floating around.
2. Typically you'd call assume_init immediately after doing the initialization, before doing anything that might create more references.

Of course, it would still be good to put this non-obvious gotchas, especially around Pin, in the docs. Also, I think you can technically do the initialization without UB using just shared references and pointer casts (i.e., without checking refcount==1) if you involve UnsafeCell, but that seems very convoluted and I don't know of any case where it's useful or easier than the &mut MaybeUninit<T> route. In the Arc case it would also likely risk UB by data races if there's unknown other references around.

[Diggsey]

Right - I think the std library should be opinionated here - ie. either say that it's an error to call assume_init when multiple references exist (and verify this in debug) or say it's allowed but document the behaviour (that last Rc to drop determines drop behaviour)