When some functions require Arc<Self> due to tokio::task::spawn_blocking, what's the best practice?

[jbe]

Consider the following example:

use tokio::task::spawn_blocking;

use std::sync::Arc;

struct X;

impl X {
    pub async fn new_x() -> Self {
        X
    }
    pub async fn new_arc() -> Arc<Self> {
        Arc::new(X)
    }
    pub async fn foo1(self: Arc<Self>) {
        spawn_blocking(move || self.foo_blocking()).await.expect("spawned task failed")
    }
    pub async fn foo2(self: &Arc<Self>) {
        let this = Arc::clone(self);
        spawn_blocking(move || this.foo_blocking()).await.expect("spawned task failed")
    }
    fn foo_blocking(&self) {
        /* potentially blocking */
    }
}

struct Y {
    inner: Arc<X>,
}

impl Y {
    pub async fn new() -> Self {
        Y { inner: Arc::new(X) }
    }
    pub async fn foo3(&self) {
        let inner = Arc::clone(&self.inner);
        spawn_blocking(move || inner.foo_blocking()).await.expect("spawned task failed")
    }
}

#[tokio::main]
async fn main() {
    // variants 1 and 2: manually wrap `X` into an `Arc`
    let x_manual_arc = Arc::new(X::new_x().await);
    x_manual_arc.clone().foo1().await; // variant 1: `foo` consumes `Arc<Self>`
    x_manual_arc.foo2().await; // variant 2: `foo` takes `&Arc<Self>`
    
    // variants 3 and 4: make `new` return an `Arc<Self>`
    let x_arc = X::new_arc().await; // variants 3 and 4
    x_arc.clone().foo1().await; // variant 3: `foo` consumes `Arc<Self>`
    x_arc.foo2().await; // variant 4: `foo` consumes `Arc<Self>`

    // variant 5: hide `Arc` as an implementation detail by making
    // `new` providing a wrapper around a private inner value
    let y = Y::new().await;
    y.foo3().await;
}

(Playground)

I wonder which of the variants 1 through 5 is the best way to go?

I feel like variant 1 is the most idiomatic one to use because it won't (force) unnecessary creation of Arcs. But it does feel somewhat unergonomic due to the extra Arc::new and Arc::clone needed when calling the foo method:

    let x_manual_arc = Arc::new(X::new_x().await);
    x_manual_arc.clone().foo1().await;

[H2CO3]

I'd also say that making the Arc creation explicit is the better option, not because it's explicit, but because it doesn't force an unwrapping operation in the case when you need ownership of the value (without the Arc), increasing type safety (since Arc-unwrapping is fallible).

Taking the Arc by-value is also the more expressive option in that it shows clearly that ownership of the Arc is needed (and it can also avoid an unnecessary clone, although cloning Arc is cheap so this is not the foremost reason).

[jonh]

Take time to code as not to need Arc and spawn_blocking.

Assuming the above isn't chosen;
Consider what method will cause least breakage if code refactored in future. Hiding the Arc internally generally will do so; but I have no idea if Arc is needed outside to be used in other parts of code.

[jbe]

@jonh

Take time to code as not to need Arc and spawn_blocking.

In my code I need spawn_blocking because of this:

@jbe

Note that default_net::interface::get_interfaces seems to be a somewhat expensive operation.

Maybe I should consider this to be an (ugly) implementation detail and go for variant 5. But in the particular case, it will be a private module, so I don't need to care for a clean/stable API.

---

Anyway, I believe that in Rust it's usualluy idiomatic to expose the question whether an Arc is needed/consumed/created to the user of an API.

[alice]

If all you need is to call get_interfaces, then you can probably do this:

let interfaces = spawn_blocking(|| {
    default_net::interface::get_interfaces()
}).await;

This way, you don't have to clone self into the spawn_blocking closure.

[jbe]

Oh right, so something like this:

impl X {
    pub async fn new() -> Self {
        X
    }
    pub async fn foo(&self) {
        let interfaces = spawn_blocking(get_interfaces).await.expect("spawned task failed");
        self.foo_implementation(&interfaces);
    }
    fn foo_implementation(&self, _interfaces: &Interfaces) {
    }
}

#[tokio::main]
async fn main() {
    let x = X::new().await;
    x.foo().await;
}

(Playground)

(And maybe move the implementation inside foo too.)

So I guess I made my (underlying) problem unnecessarily complex (and it can be simplified to not need the Arc). Thanks!