Sharing MutexGuard between functions in the same struct

[xpepermint]

I use std::sync:Mutex to share an object between threads. In each thread, the guarded object must be accessed on several places thus almost every function needs to do a lock().unwrap() to perform actions.

In the example below I try to illustrate the issue so please no, it does not make (real) sense.

pub struct Stage {
    foo: String,
    bar: String,
}

pub struct Worker {
    stage: Arc<Mutex<Stage>>,
}

impl Worker {
    fn read_foo(&mut self) -> String {
        let mut stage = self.stage.lock().unwrap();
        stage.foo.clone()
    }
    fn read_bar(&mut self) -> String {
        let mut stage = self.stage.lock().unwrap();
        stage.bar.clone()
    }
}

impl FuturesStream for Worker {
    type Item = String;
    fn poll_next(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Option<Self::Item>> {
        let foo = self.read_foo();
        let bar = self.read_bar();
        ...
    }
}

This looks redundant to me and I was thinking about creating a single call to lock().unwrap() and then pass it to each the function:

pub struct Stage {
    foo: String,
    bar: String,
}

pub struct Worker {
    stage: Arc<Mutex<Stage>>,
}

impl Worker {
    fn read_foo(&mut self, stage: MutexGuard<Stage>) -> String {
        stage.foo.clone()
    }
    fn read_bar(&mut self, stage: MutexGuard<Stage>) -> String {
        stage.bar.clone()
    }
}

impl FuturesStream for Worker {
    type Item = String;
    fn poll_next(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Option<Self::Item>> {
        let mut stage = self.stage.lock().unwrap();
        let foo = self.read_foo(&stage);
        let bar = self.read_bar(&stage);
        ...
    }
}

I also wonder if I could make a sharable mutex lock where each function would make a lock if the guard is not present like this:

pub struct Worker {
    stage: Arc<Mutex<Stage>>,
    lock: Option<MutexGuard<Stage> + 'static>,
}

impl Worker {
    fn acquire_stage(&mut self) -> &MutexLock<Stage> + 'static {
        if self.lock.is_none() {
            self.lock = Some(self.stage.lock().unwrap());
        }
        &self.lock
    }
    fn release_stage(&self) -> MutexLock<Stage> + 'static {
        self.lock = None
    }
    fn read_foo(&mut self, stage: MutexGuard<Stage>) -> String {
        self.acquire_stage().foo.clone()
    }
}

I wonder, is this "optimization" a valid step or I try to go too far? I try to minimize the execution time and as far as I understand how Mutex works, the locking adds one or more ms to the execution time so why not just have a single lock call then. What is the best practice here? How would you do that? Is there maybe a way to do a mutex-like data sharing without locking?

[raidwas]

Off the top of my head, you could write a function for the Worker that takes a closure, aquires the lock and calls the closure with a reference to the Stage.

(I'm on mobile right now so forgive me for no proper formatting).

[Riateche]

You can't put a MutexGuard in the same struct with Mutex it refers too. That would create a self-referential struct, and that is generally not what you want in Rust (although there are workarounds).

In this case, I would try to avoid passing self to read_foo and read_bar. You can pass a &Stage or a &mut Stage and some other data they need. This would allow you to write code like that:

     fn poll_next(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Option<Self::Item>> {
        let mut stage = self.stage.lock().unwrap();
        let foo = read_foo(&stage, self.something_else);
        let bar = read_bar(&stage, self.something_else);
        ...
    }