Fight with borrow checker about structure that changes its own state

help
#1

I'm relatively new to Rust. I have written the code below.

The idea is that a new Client is "uninitialized" (log_in() hasn't been called), but certain methods (get_data() in this example) lazily do that before they call any other Connection methods. There is also some data foo that is stored in the Client and is needed by get_data().

struct Connection {}
impl Connection {
    fn log_in(&self) { println!("Logging in"); }
    fn get(&self, foo: &str) { println!("Requesting data (foo={})", foo); }
}

struct Client {
    connection: Option<Connection>,
    foo: String,
}

impl Client {
    fn new(foo: &str) -> Client {
        Client { connection: None, foo: String::from(foo) }
    }
    fn connection(&mut self) -> &Connection {
        if self.connection.is_none() {
            let c = Connection{};
            c.log_in();
            self.connection = Some(c);
        }
        &self.connection.as_ref().unwrap()
    }

    fn get_data(&mut self) {
        self.connection().get(&self.foo)
    }
}

pub fn main() {
    let mut client = Client::new("foofoo");
    client.get_data();    
}

Playground link

Unfortunately it doesn't work as I intended:

26 |         self.connection().get(&self.foo)
   |         ----              --- ^^^^^^^^^ immutable borrow occurs here
   |         |                 |
   |         |                 mutable borrow later used by call
   |         mutable borrow occurs here

It seems that after calling connection() (the call has already returned) I can't access foo anymore?

I guess I have to approach this problem completely different?

On a side note, it also feels weird that any variable holding a Client needs to be mut, but I guess that's a good thing?

#2

The error is because connection borrows it mutably, and thus nothing else can access it until that borrow ends.

#3

And that borrow doesn't end when connection() returns?

#4

And can I restructure my struct to avoid that problem?

#5 
fn get_data(&mut self) {
        let foo = (&self.foo).to_owned();
        self.connection().get(&foo)
    }

wouldn't that work as a workaround for now?

#6

That would really just be a workaround.

See, all the methods of Client basically call connection() to lazily initialize the Client and then call various methods of Connection with various parameters from Client. If I have to copy each of those parameters before I can use them, it feels like there must be a better structure.

I'm doing this lazy initialization because making a network request in new() feels just wrong.

Maybe I should have a login() method and force users of Client to call it before each method call? But that feels like redundant code for something that Client could do on its own.

#7

why do you need to return the Connection from the connection method? Is that a requirement?

#8

Not a requirement, I just thought it makes it more convenient. I considered renaming it to init() and removing the return value, but then the methods of Client would look like this

fn some_client_method(&mut self) {
    self.init();
    self.connection.unwrap().some_connection_method();
}

which felt boilerplate-y.

#9

I think it's better to just do that than fight rust's ownership rules :sweat_smile: I'm sure there's some way to do what you want, But It'd probably be unnecessary.

maybe try a refcell somewhere in there to get rid of the mutable borrow? I don't have my laptop at hand or I'd give it a go.

#10

What about calling your constructor something like connect() instead? Nothing mandates the name new(), and it doesn’t feel right to me for the object to not be ready for use when the constructor returns.

If you do decide to keep the lazy initialization, consider using interior mutability to store the connection (like inside a RefCell or RwLock) so that you don’t need every method to take &mut self.

2 Likes
#11

Move the abstraction to the type / field that involves it: you are lazily initializing a Connection, so it looks like you should have a LazyConnection wrapper around Connection that does that, but then only interacting with the .connection field specifically, thus letting Rust see that .foo is indeed not borrowed:

#[derive(Default)]
struct LazyConnection /* = */ (
    Option<Connection>,
);

impl LazyConnection {
    fn get (self: &'_ mut LazyConnection)
      -> &'_ mut Connection
    {
        self.0.get_or_insert_with(|| {
            let /* mut */ c = Connection { /* ... */ };
            c.log_in();
            c
        })
    }
}

struct Client {
    connection: LazyConnection,
    foo: String,
}

impl Client {
    fn new (foo: &'_ str) -> Client
    {
        Client { connection: LazyConnection::default(), foo: String::from(foo) }
    }

    fn get_data (self: &'_ mut Client)
    {
        self.connection.get().get(&self.foo);
    }
}

fn main ()
{
    let mut client = Client::new("foofoo");
    client.get_data();    
}

For the very case of lazy initialization, there are some nice external crates out there that already provide a nice API for it, see ::oncecell

3 Likes
#12

Beat me to it :slight_smile:. Was just about to post an uglier version of this concept. I'm assuming no one is dying to see a less elegant version.

1 Like
#13

@drewkett I don't think the solutions to this kind of general pattern-related problems are that unique, so any kind of solution would be welcome :slightly_smiling_face:

Even if yours were to be similar to the one I posted, that would still lead to two slightly different things to achieve the same pattern, which can be useful to better grasp, by comparison, what the pattern really is.

#14

Correct. This is because connection returns a reference. The mutable borrow of self does not end until the returned reference is no longer used.

1 Like
#15 
use once_cell::sync::Lazy;
struct Connection {}

impl Connection {
    fn log_in(&self) {
        println!("logging in");
    }
    fn get(&self, foo: &str) {
        println!("requesting data (foo={})", foo);
    }
}

struct Client {
    connection: Lazy<Connection>,
    foo: String,
}

impl Client {
    fn new(foo: &str) -> Client {
        Client {
            connection: Lazy::new(|| {
                let c = Connection {};
                c.log_in();
                println!("logged in");
                c
            }),
            foo: String::from(foo),
        }
    }

    fn get_data(&self) {
        println!("getting");
        self.connection.get(&self.foo)
    }
}

pub fn main() {
    let client = Client::new("foofoo");
    println!("client created, getting data");
    client.get_data();
}

Playground Link
The OnceCell version works really well. I can't stress enough how useful OnceCell has been to me on various occasions

Output

client created, getting data
getting
logging in
logged in
requesting data (foo=foofoo)
1 Like
#16

How about decoupling the connection function from self and using the decoupled function?

fn connection(&mut self) -> &Connection {
    Self::connection_inner(&mut self.connection)
}

fn connection_inner(connection: &mut Option<Connection>) -> &Connection {
    if connection.is_none() {
        let c = Connection{};
        c.log_in();
        *connection = Some(c);
    }
    &connection.as_ref().unwrap()
}

fn get_data(&mut self) {
    let Self { connection, foo } = self;
    Self::connection_inner(connection).get(foo)
}

Playground

1 Like
#17

So many good solutions, thanks. I'll certainly refer to this in the future.

In this particular case I'll just do this:

Very simple and it just didn't occur to me to do it like that.

Oh, indeed, returning an immutable reference from a mutable borrow does not "downgrade" the borrow. (Playground link) I didn't know that.