Need review of code that uses rocket and mutex

This is my first attempt at creating a Rocket API server and my first time using std::sync::Mutex. I'd love to get some feedback on whether my approach can be improved. It's relatively short (less than 100 lines of actual code) and implements CRUD operations on a collection of structs that describe dogs.
See rust-rocket-demo/main.rs at master · mvolkmann/rust-rocket-demo · GitHub. Once I get this vetted, maybe it can serve as a good example for others new to Rocket.

It seems reasonable at a first glance.

Please be aware that if you ever make any of the routes an async fn, you need to be aware of some details when using the std mutex. You can read about these details here. This is not to say you are doing anything wrong now.

3 Likes

The dogs don't seem to need to be mutable, so you might want to instead store Arc<Dog> to reduce the cost of cloning them.

1 Like

I assume you are referring to the create_dog and update_dog functions.
I haven't used Arc yet.
Could you show me what the code would look like in one of those functions if I use that instead of cloning?

I'm referring to the storage itself, you could store a HashMap<String, Arc<Dog>>. You'll just have to sprinkle in a few Arc::news to create the Arcs, but everything else should mostly be the same. And of course return Arc<Dog> from the functions instead of just Dog.

I suspected my attempt to learn how to use Arc would fail and I was right. :wink:
Can you look at this and tell me what I'm doing wrong with Arc?

I believe the return values should be Json<Arc<Dog>>.

Are you saying I should change the return type specified for the create_dog and update_dog functions? I tried changing those to Json<Arc<Dog>>, but that gives the error "the trait _::_serde::Serialize is not implemented for std::sync::Arc<Dog>" which I think makes sense.

I see. That's unfortunate. Maybe the best is to just clone it and not use Arc.

You could get around it by writing a wrapper struct that contains an Arc<Dog> and forwards the Serialize impl to the Dog.

I'm sure for this simple app cloning is fine. But I figured this was an opportunity to learn how to use Arc since I haven't done that yet. Maybe @Kestrer can point me in the right direction.

The impl would look like this:

use std::sync::Arc;
use serde::Serialize;
use serde::ser::Serializer;

struct SerializableArc<T> {
    inner: Arc<T>,
}

impl<T: Serialize> Serialize for SerializableArc<T> {
    fn serialize<S: Serializer>(&self, ser: S) -> Result<S::Ok, S::Error> {
        self.inner.serialize(ser)
    }
}

Then you should be able to return Json<SerializableArc<Dog>>.

Couldn't I instead get the inner value out of the Arc in the call to Json instead of making the Arc serializable? Something like this? Json(dog.get_wrapped_value()) Of course get_wrapped_value is not a real method of Arc, but does such a thing exist? I don't see one.

You can do that, yes. The method is (&*the_arc).clone(), the &* here getting an &Dog from the Arc<Dog>, which is then cloned (instead of calling the Arc::clone method). Or more explicitly, you can do Dog::clone(&the_arc).

However, this method defeats the purpose of using an Arc, namely to avoid the clones.

You can avoid the wrapper entirely if you enable the rc feature of Serde, which will add Serialize/Deserialize implementations for Rc<T> and Arc<T> - see the serde docs.

1 Like

I feel like this is getting too complicated. I can't figure out the exact syntax on my own. Maybe I need to go back to just creating clones.

The suggestion of enabling the rc feature should be very simple. You just update the Cargo.toml to have

[dependencies]
serde = { version = "1", features = ["rc"] }

then do the Json<Arc<Dog>> thing from before.

1 Like

Off topic but Json<Arc<Dog>> is my new favorite Rust type name.

2 Likes

Okay, I tried that and updated the code to use @Cole-miller 's favorite type. :wink:
What do I need to do to fix this line?

The error I get is "cannot move out of *dog which is behind a shared reference
move occurs because *dog has type std::sync::Arc<Dog>, which does not implement the Copy trait"
I know I don't want to copy anything because that was the point of using Arc.

Use Some(Json(dog.clone())). The point of Arc and Rc is that .clone() is a cheap operation that doesn't involve copying any of the underlying data -- it just increments a reference count.

1 Like

Some people like to write this as Some(Json(Arc::clone(dog))) to make it clear that this is only a pointer copy and not a “deep” copy.

1 Like