Beginner Code review: How to implement a while loop with mutable reference to self?

What is the correct way to implement the while loop with mut ref to self?

    pub fn run(&'a mut self) {
        while !self.stop_running {
            self.run_quants();
        }
        self.finished();
    }

    fn run_quants(&'a mut self) {
        while self.dispatch_current_quant_events() {
          for (_, environment) in self.environments.iter_mut() {
            environment.reset_entity_limits();
          }
        }
    }

ERROR 1: cannot use self.stop_running because it was mutably borrowed
ERROR 2: cannot borrow *self as mutable more than once at a time

This is my whole code

1 Like

Why is there a lifetime on &'a mut self? I assume this lifetime appears somewhere on the type?

The type &'a mut Type<'a> is almost never correct. It implies that every time you call the function, you mutably borrow the object for the rest of its life.

(in fact, the only use case I know of for writing such a function signature is to deliberately render an object impossible to use afterwards)

2 Likes

I see. So maybe I just have implemented my lifetimes wrong?
Both functions belong into impl block of Supervisor that has lifetime of 'a. The lifetime is followed also by the Environment, Event Struct and Entity struct that are all dependent on each other.

    pub struct Supervisor<'a> {
        environments: HashMap<String, Environment<'a>>,
        events: Vec<Event<'a>>,
        entities: Vec<Entity<'a>>,
        stop_running: bool,
        pub current_quant: usize
    }

    impl <'a> Supervisor<'a> 
    {
        pub fn run(&'a mut self) {
            while !self.stop_running {
                self.run_quants();
                // self.sleep(200);
            }
            self.finished();
        }

        fn run_quants(&'a mut self) {
            // keep running as long as there are still events in the queue
            while self.dispatch_current_quant_events() {
              // reset all invocation limits for the next quant
              for (_, environment) in self.environments.iter_mut() {
                environment.reset_entity_limits();
              }
            }
        }
    }

What should I do then? What other information I can share with you to get this right?

What are Event and Environment borrowing data from? Could you clone the data that is being borrowed instead? (it’s better to miminize your interaction with the borrow system early on if possible until you gain a better intuition for it)

Event holds the code to be run when calling .disptach(), Environment is a group of Entities that trigger Events.

I would like to learn to work with Rust the right way from the start. Clone() would be like giving up and using band-aid. From what I know it is slow, wasteful (duplicates in memory) on resources and can cause problems where I need to keep only a single instance of an object.

Getting lifetimes right is a time consuming endeavor that isn’t always worth it. It is perfectly fine to clone data and later refactor to use references instead.

Lifetimes isn’t the only thing Rust has to offer, so you can focus on the many other aspects of Rust before jumping into lifetimes.

Another option is to use reference counted pointers like Rc/Arc, this will make clones cheap and you avoid the lifetime issues. Later you can refactor to use references id their cost is too much, but it should be fine the vast majority of cases.

2 Likes

Event holds the code to be run when calling .disptach(), Environment is a group of Entities that trigger Events.

Okay, the specific thing I’m trying to figure out here is: Do the things in entities borrow data from the things in events? (or etc. with any two fields) Or do all of these things exclusively borrow data that outlives Supervisor?

I would like to learn to work with Rust the right way from the start. Clone() would be like giving up and using band-aid. From what I know it is slow, wasteful (duplicates in memory) on resources and can cause problems where I need to keep only a single instance of an object.

Part of the thing is, borrows are not always the right way. They’re a performance optimization that the compiler tries to help you get right with brutal discipline. Typical rust code should not be full of explicit lifetimes.

(I say “explicit” because borrows are extremely common as input arguments, and as output of “getter” functions; but the default lifetimes inferred by the rules of lifetime elision serve 99% of these cases)

1 Like

The are all dependent on eachother :slight_smile: Supervisor lives the longest so all other lifetimes should be aligned with that.

pub struct Environment<'a> {
    entities: Vec<&'a mut Entity<'a>>,
    id: String,
    name: String,
    pub type_info: Option<TypeStmt>
}

pub struct Entity<'a> {
    supervisor: &'a mut Supervisor<'a>,
    effects: Vec<Effect<'a>>,
    id: String,
    invoked: usize,
    limit: usize
}

pub struct Event<'a> {
    pub entity: &'a mut Entity<'a>,
    pub quant: usize,
    pub value: TritVector
}

pub struct Effect<'a> {
    pub env: &'a mut Environment<'a>,
    pub delay: usize,

}

pub struct Supervisor<'a> {
    environments: HashMap<String, Environment<'a>>,
    events: Vec<Event<'a>>,
    entities: Vec<Entity<'a>>,
    stop_running: bool,
    pub current_quant: usize
}

Im all ears if you have a better solution for me, if I can get around using lifetimes and hopefully do not clone everything all the time :slight_smile:

You can see my entire code here…

Okay, so it looks like you are trying to construct a self-referential type. This is trouble; for a simple example:

// assume supervisor: Supervisor
let renamed = supervisor;

Here, when we simply move a supervisor through assignment, its location in memory may change, causing the supervisor field of all of its Entitys to become dangling pointers.

More generally, rust has an extremely difficult time with objects that contain a reference that point anywhere into any field on that same object. Even in cases where the referent should have a stable address (e.g. you have an &i32 borrowing from a Box<i32>), rust’s lifetime system isn’t expressive enough for the compiler to realize that it is safe.

(there’s also another huge problem here: Supervisor is aliased, because there are many Entities that all have mutable borrows pointing into it)

The two simplest pieces of advice to keep in mind are:

  • Borrows should occur in stages.

    • As soon as you borrow something, it is anchored in place. You can’t move it. You can’t modify it (other than through the borrow you just made, if it was mutable). You might not even be able to look at it anymore.
    • Therefore, if type C borrows from type B, and type B borrows from type A, it’s usually easiest to make all of the As first, then all of the Bs, then all of the Cs. (otherwise, problems will occur when you add a new object to your collection of As)
  • The “parent object pointer” pattern from OOP is impossible.

    • It’s not clear to me yet what Entity would have used supervisor for, but whatever it is, that function should take whatever it needs from the supervisor as an argument. (or maybe it should even be defined somewhere else!)
    • You might find this objectionable coming from OOP, but remember that the unit of encapsulation in Rust is the module, not the type. A type doesn’t necessarily need to be capable of doing anything useful on its lonesome. (it could just a bundle of data to be used by another type or a function)
3 Likes

Entity asks Supervisor for list of Environments.

So you mean instead of structs I should be using mods? Can they have multiple instances? Or what do you mean by that last one exactly?

I am not sure I understand what you are saying. Perhaps some sample code of how you would build one of my structs?

You seem to be using references as if they were equivalent to Java-like using of objects by reference. They’re not. References in Rust are compile-time locks. Your &muts tell the borrow checker to treat them as temporary and locked for exclusive access.

If you want Java-like reference to “I can use this object from anywhere”, the type for it in Rust is Arc<Mutex<T>> (e.g. Arc<Mutex<Supervisor>>). Single-threaded version of it is Rc<RefCell<T>>.

You need refcounting, because your objects form an arbitrarily complex graph where anything can access anything else.

If you changed it to be strictly a tree, you could use “normal” owned objects without extra wrappers, lifetimes, etc.

4 Likes

Hmmm… very interesting! :slight_smile: So If I wrap all my variables currently referenced variables into Arc<Mutex I do not need to use lifetimes anymore?
Would you mind making a sample code snippet how would you implement this in my context eg Environment

For the simple case, where there is no intent of mutating the pointee, &'a T becomes Arc<T> within your structs.

So, where you used to create a value x of type T,

let x: T = ...;

You now need to wrap the value in an Arc:

let x: Arc<T> = Arc::new(...);

And now instead of giving a borrow of x, like &x, you give an Arc::clone(&x).
The code becomes slightly more cumbersome, but you get rid of references this way :slight_smile:

If you want to ever be able to mutate a value with this pattern, you need to wrap it in a RwLock (or a Mutex) before wrapping it in the Arc: Arc<RwLock<T>>.

Sharing it is still done using the .clone() from the outer Arc, but now reading the pointee requires calling .read().unwrap(), and read/writing to it requires .write().unwrap().


For a brief summary of these wrapper types, see this post

2 Likes
use std::sync::{Arc, Weak, Mutex};

type Ptr<T> = Arc<Mutex<T>>;
type WeakPtr<T> = Weak<Mutex<T>>;

pub struct Environment {
    entities: Vec<Ptr<Entity>>,
    id: String,
    name: String,
    pub type_info: Option<TypeStmt>
}

pub struct Entity {
    supervisor: Ptr<Supervisor>,
    effects: Vec<Effect>,
    id: String,
    invoked: usize,
    limit: usize
}

pub struct Event {
    pub entity: Ptr<Entity>,
    pub quant: usize,
    pub value: TritVector
}

pub struct Effect {
    pub env: Ptr<Environment>,
    pub delay: usize,
}

pub struct Supervisor {
    environments: HashMap<String, Ptr<Environment>>,
    events: Vec<Ptr<Event>>,
    entities: Vec<Ptr<Entity>>,
    stop_running: bool,
    pub current_quant: usize
}

You may want to use WeakPtr in some place where Ptr is used to get rid of cycles, but I can’t tell where to put them from just this. If you do end up with cycles everything in the cycle will be leaked, so be careful to not create cycles.

You can read about Arc, Weak and Mutex in their docs as linked here. tldr:

Arc gives you shared ownership, which is similar to normal Java pointers, but Arc prevents mutation by default because shared mutation is hard. Cloning an Arc gives you a new handle to the same data, and bumps a ref-count. This makes Arc::clone very cheap.

Mutex is a special type that safely allows shared mutation and so by using it with Arc we can regain mutation to behave like Java pointers.

Weak is like Arc but it doesn’t own what it points to and so it can’t access that data directly, but Weak can be upgraded to an Arc to gain access to the underlying data.

2 Likes

Amazing Yandros! Thank you so very much. This is exactly what I needed! :slight_smile:

In your example you do use & sign in front of x Arc::clone(&x) .
Isn’t that still a reference?

Thank you so much for this sample code Krishna Sannasi, I will read up on Arc, and Mutex and try them out :slight_smile: Would be amazing if I can get rid of lifetimes and these limitations I have been struggling with :slight_smile:

I didnt quite get, in which cases would it make sense to use Weak instead of Arc<Mutex<>> ?

But lifetimes are what makes Rust effective and safe :slight_smile:

I think the most obvious case would be the Entity's supervisor field. There could be a cycle where

supervisor_a owns entity_a in supervisor_a.events
and
entity_a owns supervisor_a in entity_a.supervisor

and so there is a cycle, this cycle will leak supervisor_a and entity_a as well as everything both of them own. To break this cycle, you can instead make Entity's supervisor field a WeakPtr<Supervisor> this way Entity doesn’t own Supervisor. This way there is no cycle and everything gets dropped correctly.

You can see an example of a cycle here

1 Like

Someone just said above that I shouldn’t touch lifetimes until i am a full on Rust PRO :smiley:

If I use the Weak Pointer, can I still call it’s methods? Eg, I need to call .list_environments() on supervisor from the Entity

You can temporarily upgrade a Weak to a Arc with Weak::upgrade but don’t store that Arc, just use it to call methods and then let it drop. Storing that Arc may cause leaks.