Preventing `Rc` leaks when it references the stack

[matthieum]

Introduction

There has been quite an outcry over the issue with Rc and thread::scoped, and two RFCs that I know of to try and “improve” the situation:

• RFC 1085: Leak and Destructor Guarantees
• RFC 1094: Guaranteed non-static destructors

However both have downsides:

• The introduction of a new marker trait (Leak) unfortunately introduces yet another orthogonal axis along which the world splits
• Running destructors in cycles of references is complicated (Python does not guarantee destructors will run precisely because of cycles)

Therefore I felt compelled to propose yet another alternative, one which would not affect the language, but would affect Rc (and Arc). This is a work in progress, and I was hoping the community could help me shape it up.

Gist

Change the bound on Rc and Arc to include 'static, and thus forbid references to the stack.

Since that would actually a tad too restricting, it also presents a simple piece of code that shifts the lifetime check from compile-time to runtime, with minimal usage of unsafe code, allowing Rc to regain its usefulness while guaranteeing that references to the stack cannot outlive the items they reference even in the presence of cycles (and leaks).

How to?

I got the idea when reading Niko’s article about how MutexGuard did not have the issue than JoinGuard had because it had a runtime check and would leave the actual Mutex poisoned.

From then on, the question is: how to poison the stack? The implementation can be seen on the playpen.

I use 3 simple structures:

struct BorrowGuard<'a, T: 'a>
    where T: 'a
{
    reference: &'a T,
    count: Cell<usize>,
}

struct BorrowAnchor<'a, 'b, T>
    where 'a: 'b, T: 'a
{
    reference: &'b BorrowGuard<'a, T>, // should be mut, to guarantee the anchor's unicity
}

struct BorrowRef<T>
    where T: 'static
{
    reference: &'static T,
    count: &'static Cell<usize>,
}

The Guard itself borrows the target object, guaranteeing it will outlive its references. The Anchor borrows the Guard, guaranteeing that its Cell will be immovable. Finally, the Ref strips the lifetimes, and can therefore be used in Rc, among other things.

Note: at this point, one realizes that we would need 2 guard/anchor/ref triplets, as another would be needed for mutable references; I only present one such triplet here for simplicity.

It actually works pretty much like Rc: Guard::count always represents the number of existing Ref at any point in time (briefly out-of-sync during construction/drop of a Ref) and it is up to the user to guarantee that all Ref were dropped before the Anchor itself is dropped, if the user fails in that, Anchor aborts in its implementation of Drop.

Good Points

• Performance: I would expect the performance impact to be relatively minimal, the counter is only incremented/decremented when a Ref is formed or destroyed (which the user controls) and it is only checked when the Anchor is destroyed.
• Freedom: A user can freely use Rc or Arc without any additional burden.

Bad Points

• Aesthetics: Obviously, an aesthetics hit. One if forced to form both a Guard and an Anchor before being able to have any Ref, and because the Guard outlives the Anchor (if only briefly) I see no easy way to make this more lightweight (as a single tuple). As such, embedding a reference to the stack into Rc or Arc requires more setup than it does today.
• Narrowness: Cycles are only detected if there are references to the stack, so it does not prevent all cycles.

Comments?

I would like to hear about the community on this. It seems to me that while there is an impact on the usability of Rc and Arc, it is worth it to prevent leaks as much as possible.

The idea of adding the 'static to Rc and Arc, while it would have prevented the scoped fiasco, was not popular due to the fact that keeping references to the stack is expected for efficiency reasons, however this Guard/Anchor/Ref dance rehabilitates it without requiring the user to reach toward unsafe by herself.

[glaebhoerl]

It seems to me that this could complement, rather than replace, #1094…?

[matthieum]

Actually it is unclear whether it could replace #1094, it uses its basic premise (leaking Rc is safe if they only refer 'static data) at the very least.

It is just that the design of ScopedRc with its cycle registry mechanism is rather heavyweight and I was looking for a leaner alternative, which I think I managed (though it’s unproven code…).

On the other hand, I would not be surprised if there were cases for which ScopedRc would be strictly superior, either because it can actually express those cases where the mechanism I present cannot or because its usage would be easier.