Code cleanup issue 1

help
1

Having got a fairly large app working, (around 23 files) I've been cleaning it up and trying to understand some of the things which I just did to make it compile without real understanding. I've got rid of all the warnings and all the lifetime annotations which I don't like much and turned out there were ways to avoid them. One issue I didn't understand on the way through was a use of Option. I used this to hold a join handle where I may not start the thread.
So in the Struct.

pub opt_reader_join_handle: option::Option<thread::JoinHandle<()>>,

It is in initialized to None and then if I start the thread it will have a Some.
When I close down I want to wait for the thread to exit.
This was the solution

if let Some(h) = self.opt_writer_join_handle.take(){
                println!("Waiting for writer to terminate...");
                h.join().expect("Join UDP Writer failed!");
                println!("Writer terminated");
            }

Which I don't like. It only works because I am closing so don't need it again but if I did I can't take() it twice.

This is what seems sensible

match &self.opt_writer_join_handle {
                Some(inst) => inst.join().expect("Join UDP Writer failed!"),
                None => ()
            }

but does not compile.
error[E0507]: cannot move out of *inst which is behind a shared reference
--> src\app.rs:283:31
|
283 | Some(inst) => inst.join().expect("Join UDP Writer failed!"),
| ^^^^^------
| | |
| | *inst moved due to this method call
| move occurs because *inst has type JoinHandle<()>, which does not implement the Copy trait
|
note: this function takes ownership of the receiver self, which moves *inst

I've had this issue with takes ownership of self many time in different circumstances and I still don't know the proper solution. If in the example it was not in an Option I'm sure there would be no issue as the take() demonstrates.

2

join() needs ownership, so you have to give up ownership of the handle if you want to join the corresponding thread.

You can't join() a thread multiple times, which is what the join() method taking ownership indicates. So this has nothing to do with Option and everything to do with the semantics of joining a thread.

3 Likes
3

Fair enough. It was a bad example. I know I've had this 'taking ownership' error a lot when calling methods on my instances. I will see if I can find a better example.

4

If you don't actually need ownership, then you can turn a &Option<T> into an Option<&T> or a &mut Option<T> into an Option<&mut T> using the .as_ref() or .as_mut() methods, respectively.

5

I still have a problem with this feature of Rust which seems to do something unintuitive to me to take ownership of itself.

So a method foo(self) or foo(mut self) takes ownership of self. This means if you need to store the instance it has to be through an Option and then after the take() you can no longer call any methods on that instance. I always use '&self' or '&mut self' in my code because there is never a reason to use 'self' but I know I've hit this error many times so next time I will investigate it rather than work round it and maybe understand why it happens a little better.

6

That's incorrect. If there were never a reason to take self by value, then it would (presumably) not exist as a language feature. There are several reasons to take self by-value, for example when you want to decompose a value into its parts and give away ownership of them. Say you are building some data structure using a Builder, and then you want to return the completed data structure:

#[derive(Default)]
struct VecBuilder(Vec<u64>);

impl VecBuilder {
    fn add(&mut self, number: u64) -> &mut Self {
        self.0.push(number);
    }

    fn build(self) -> Vec<u64> {
        self.0
    }
}

fn main() {
    let mut builder = VecBuilder::default();
    builder.add(1).add(2).add(3);
    let v = builder.build();
    assert_eq!(v, [1, 2, 3]);
}

In this case, the right thing to do for VecBuilder::build() to take self, because this is how it can then return the Vec by-value. If it took a &self, the only thing it could do would be to clone the entire vector and return the clone, since it couldn't move the original one out of *self.

The Option::take() workaround is pretty rare in well-designed code. If you find that you always need to use it, then your code is misdesigned and you likely don't understand what references and values are for. This is not the fault of the language.

7

This really is a difficult concept to understand. I kind of get your example but that's a short lived instance and if you want another builder you create one. For long lived instances you don't want to be giving away the handle because a new instance is no good to you.
The only time I used take() was when I had no choice to call .join().
As I said I will try and find a better example to illustrate.

8

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.