S35: Rc cycles (memory leak)

S35Severity: Medium

Two values that own each other are never freed.

Clippy: no lint finds this. Look for it in review.

What it looks like

A customer holds its orders, and each order holds its customer. Both links are Rc:

pub struct Customer {
    pub name: String,
    pub orders: RefCell<Vec<Rc<Order>>>,
}

pub struct Order {
    pub id: u64,
    pub customer: Rc<Customer>, // strong: the order keeps the customer alive
}

pub fn place_order(customer: &Rc<Customer>, id: u64) -> Rc<Order> {
    let order = Rc::new(Order {
        id,
        customer: Rc::clone(customer),
    });
    customer.orders.borrow_mut().push(Rc::clone(&order)); // and the customer keeps the order alive
    order
}

Why AI agents produce it

It is the Java object model. A JPA @OneToMany and @ManyToOne pair gives each side a reference to the other. In Java, the garbage collector frees the cycle when nothing else points to it.
It is the result of fixing borrow errors. The order needs the customer’s name. The agent first tries a reference, gets lifetime errors, and changes it to Rc<Customer>. Each step compiles.
It passes every test. A leak does not change any result. Only memory use grows.

Why it is a problem

Reference counting cannot free a cycle. Rc frees a value when its count reaches zero. The customer’s count includes each order’s pointer, and each order’s count includes the customer’s Vec. When the rest of the program drops its handles, every count is still at least one. The test worse_is_never_freed proves it with a Weak that can still upgrade.
The leak grows with use. A long-running service that builds a customer graph for each request leaks each one. Memory grows until the process restarts.
Drop never runs. If a type in the cycle closes a file or a connection in Drop, that resource is never released.
It is memory-safe, so nothing warns. Rust guarantees no use-after-free, but leaking memory is safe in Rust. The compiler and Clippy do not report it.

The better version

WorseRc in both directions. The customer and its orders keep each other alive forever.

pub struct Customer {
    pub name: String,
    pub orders: RefCell<Vec<Rc<Order>>>,
}

pub struct Order {
    pub id: u64,
    pub customer: Rc<Customer>, // strong: the order keeps the customer alive
}

pub fn place_order(customer: &Rc<Customer>, id: u64) -> Rc<Order> {
    let order = Rc::new(Order {
        id,
        customer: Rc::clone(customer),
    });
    customer.orders.borrow_mut().push(Rc::clone(&order)); // and the customer keeps the order alive
    order
}

BetterThe customer owns its orders (Rc). Each order has a Weak back-reference, and upgrade() returns an Option.

pub struct Customer {
    pub name: String,
    pub orders: RefCell<Vec<Rc<Order>>>, // the customer owns its orders
}

pub struct Order {
    pub id: u64,
    pub customer: Weak<Customer>, // a back-reference that does not own
}

pub fn place_order(customer: &Rc<Customer>, id: u64) -> Rc<Order> {
    let order = Rc::new(Order {
        id,
        customer: Rc::downgrade(customer),
    });
    customer.orders.borrow_mut().push(Rc::clone(&order));
    order
}

pub fn customer_name(order: &Order) -> Option<String> {
    order.customer.upgrade().map(|c| c.name.clone())
}

Weak does not add to the strong count, so the customer is freed when its last owner drops it, and its orders with it. customer_name returns None if the customer is already gone. The test better_is_freed proves that the customer is freed, and that the order then gets None.

The Weak fix keeps the shape of the Java design. Often a simpler design is better: one owner, and IDs instead of pointers. See S03: Rc<RefCell<T>> object graph (Java translated directly) and the gallery entry A linked or tree structure.

When it is acceptable

A strong cycle is never correct in long-running code, unless the code breaks it by hand: for example, a close() method that clears the children before the parent is dropped. That is fragile, because a panic or an early return skips it. Prefer Weak.

A deliberate leak with Box::leak for data that lives as long as the program, such as configuration that is read once at startup, is a different and acceptable pattern.

How to detect it

No Clippy lint reports Rc cycles. They are a run-time property.
Look at every struct that holds an Rc<T>, and check if T holds an Rc back to it, directly or through other types. Parent and child links, and “both sides of an association”, are the usual places.
In tests, take a Weak with Rc::downgrade, drop the last outside handle, and check that upgrade() returns None.
In a running program, memory that grows with the number of requests is a symptom.

What to tell your agent

Order and Customer hold Rc pointers to each other, which is a reference
cycle that is never freed. Make the owner hold Rc and the back-reference
hold Weak (Rc::downgrade), and use upgrade() where the back-reference is
read. Better: if one owner can hold all the data, replace the pointers with
IDs. Add a test that drops the last handle and checks Weak::upgrade() is
None.