Rust Help Needed: Issue with "cannot borrow data in a `&` reference as mutable"

Question

I am new to rust and cannot understand the issue below. I am trying to store the Trait of Animals in the vector.

My implementation is as below.


mod TestAnimal {
    use crate::stack;

    pub trait Animal {
        fn diagnose(&self) -> Result<(), stack::service::ServiceError>;
    }

    pub struct Hospital {
        animals: Vec<Box<dyn Animal>>,
    }

    static mut HOSPITAL: Hospital = Hospital { animals: Vec::new() };

    impl Hospital {
        pub fn add_animal(&mut self, animal: Box<dyn Animal>) {
            self.animals.push(animal);
        }
    }

    pub fn get_hospital() -> &'static Hospital {
        unsafe {
            return &HOSPITAL;
        }
    }
}

#[test]
fn test_hospital() {
    pub struct Cat;
    impl TestAnimal::Animal for Cat {
        fn diagnose(&self) -> Result<(), stack::service::ServiceError> {
            return Ok(());
        }
    }

    TestAnimal::get_hospital().add_animal(Box::new(Cat {}));
}

The issue I am facing is as below.

error[E0596]: cannot borrow data in a `&` reference as mutable
  --> src/main.rs:45:5
   |
45 |     TestAnimal::get_hospital().add_animal(Box::new(Cat {}));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable

[You can fix this just by making the reference mutable](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=8704420701d561c7ea095ee7da7d820e) but it's *a very bad* idea to use mutable statics. You can easily break rust's safety guarantees. If you need `unsafe {}` you are most likely doing something wrong. — mousetail, Nov 30 '22 at 14:13
See [How do I create a global, mutable singleton?](https://stackoverflow.com/questions/27791532/how-do-i-create-a-global-mutable-singleton) — Chayim Friedman, Nov 30 '22 at 14:17
To illustrate why `static mut` should be avoided - [this seemingly trivial change](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=038db86fed6073259ba2612f12fab89f) to the non-`unsafe` part of @mousetail's code instantly triggers undefined behavior. — Joe Clay, Nov 30 '22 at 15:12
I will echo use of unsafe is almost always a bad idea. If you are new to rust then it is definitely a bad idea. — ctrl-alt-delor, Nov 30 '22 at 19:55
naming methods with a get name is an anti-pattern from java-beans. Consider `new` (if creating a new instance), or `the instance` (if mono-state / singleton (also anti-patterns) — ctrl-alt-delor, Nov 30 '22 at 19:57

score 1 · Accepted Answer · answered Nov 30 '22 at 19:09

Looks like you are trying to mutate a static variable. Normally this isn't safe because multiple threads editing a value at the same time can cause issues. You can make it safe by wrapping the variable in a mutex. A mutex will ensure that only one thread can mutate the variable at a time.

mod test_animal {
    use crate::stack;
    use std::sync::Mutex;

    pub trait Animal { 0 implementations
        fn diagnose(&self) -> Result<(), stack::service::ServiceError>;
    }

    pub struct Hospital { 1 implementation
        animals: Vec<Box<dyn Animal + Send>>,
    }

    pub static HOSPITAL: Mutex<Hospital> = Mutex::new(Hospital {
        animals: Vec::new(),
    });

    impl Hospital {
        pub fn add_animal(&mut self, animal: Box<dyn Animal + Send>) {
            self.animals.push(animal);
        }
    }
}

#[test]
fn test_hospital() {
    pub struct Cat;
    impl test_animal::Animal for Cat {
        fn diagnose(&self) -> Result<(), stack::service::ServiceError> {
            return Ok(());
        }
    }

    test_animal::HOSPITAL
        .lock()
        .unwrap()
        .add_animal(Box::new(Cat {}));
}

That said, sometimes you'll find you don't actually need a static variable. Mutable static variables often make code harder to understand. You might consider doing this instead:

mod test_animal {
    use crate::stack;

    pub trait Animal {
        fn diagnose(&self) -> Result<(), stack::service::ServiceError>;
    }

    pub struct Hospital {
        animals: Vec<Box<dyn Animal + Send>>,
    }

    impl Hospital {
        pub fn new() -> Self {
            Self {
                animals: Vec::new(),
            }
        }

        pub fn add_animal(&mut self, animal: Box<dyn Animal + Send>) {
            self.animals.push(animal);
        }
    }
}

#[test]
fn test_hospital() {
    pub struct Cat;
    impl test_animal::Animal for Cat {
        fn diagnose(&self) -> Result<(), stack::service::ServiceError> {
            return Ok(());
        }
    }

    let mut hospital = test_animal::Hospital::new();
    hospital.add_animal(Box::new(Cat {}));
}

Thank you all for your valuable suggestions. @bddap In your second solution how do I restrict multiple instances of the hospital? My intention was to make it a singleton. Is there a better way to make this a singleton? — Daniel, Dec 01 '22 at 16:39
If you need a singleton then the first example is what you should use. — bddap, Dec 01 '22 at 17:25
The second example is more general advice for writing software. Shared mutable state (like a mutable singleton) can make a program harder to understand as it gets bigger. — bddap, Dec 01 '22 at 17:34

Rust Help Needed: Issue with "cannot borrow data in a `&` reference as mutable"

1 Answers1