Possible pitfalls for unsafe impl<'a> Send for NotSend<'a> {}

[nduchenk]

Hi everyone. I think this question was asked million times but I can't find answer, sorry for this. What are possible pitfalls for implementing
unsafe impl<'a> Send for NotSend<'a> {}

here is a little example:

#[derive(Default, Debug)]
struct NotSend<'a> {
    something: HashMap<&'a str, &'a str>,
}

#[tokio::main]
async fn main() {
    let mut example = NotSend::default();

    let local = "local".to_owned();

    example.something.insert("key1", "static");
    example.something.insert("key2", local.as_str());

    println!("{example:?}");

    // Won't work: local does not live long enough.
    tokio::spawn(async move {
        println!("{example:?}");
    });

    tokio::time::sleep(Duration::from_secs(1)).await;
}

Even if I do implement Send for NotSend struct I won't be able to move it to task since local variable lifetime is not static.
Thanks in advance.

[alice]

Your NotSend struct is Send. The problem is the lifetimes, not whether its thread-safe. You will need to remove the lifetimes by using e.g. String instead of &'a str.

[nduchenk]

Oh, you are right, really sorry, it is wrong example, here NotSend is actually Send.
I do not need to move it to task, originally it comes from async_trait and Value from valuable crate with valuable feature for tracing.
Let me add more specific example:

use async_trait::async_trait;
use std::collections::HashMap;
use std::fmt::Debug;
use valuable::{Valuable, Value};

// unsafe impl<'a> Send for NotSend<'a> {}

#[derive(Default, Debug)]
struct NotSend<'a> {
    something: HashMap<&'a str, Value<'a>>,
}

#[async_trait]
pub trait TestTrait {
    async fn do_async(&self) -> u32;

    async fn call_do_async(&self) -> u32;
}

#[derive(Default)]
struct Service;

#[async_trait]
impl TestTrait for Service {
    async fn do_async(&self) -> u32 {
        123
    }

    async fn call_do_async(&self) -> u32 {
        let mut example = NotSend::default();
        let local = "local".to_owned();

        example.something.insert("key1", "static".as_value());
        example.something.insert("key2", local.as_value());

        println!("{example:?}");

        self.do_async().await
    }
}

#[tokio::main]
async fn main() {
    let service = Service::default();
    service.call_do_async().await;
}

So here in call_do_async fn I have errors like:
future cannot be sent between threads safely future created by async block is not Send Help: the trait std::marker::Sync is not implemented for dyn valuable::Mappable Note: future is not Send as this value is used across an await Note: required for the cast from impl std::future::Future<Output = u32> to the object type dyn std::future::Future<Output = u32> + std::marker::Send

But if I implement unsafe impl<'a> Send for NotSend<'a> {} this error is gone.
Also in code I have no control over trait, so I can't change fn signatures.

[alice]

Well, the pitfall is that you might end up moving a value from one thread to another, even though the value contains unsafe code that assumes this does not happen.

You can avoid the error by letting it go out of scope before the .await like this:

async fn call_do_async(&self) -> u32 {
    {
        let mut example = NotSend::default();
        let local = "local".to_owned();
    
        example.something.insert("key1", "static".as_value());
        example.something.insert("key2", local.as_value());
    
        println!("{example:?}");
    }

    self.do_async().await
}

[nduchenk]

But the use case will be something like this, so I can't drop example.

async fn call_do_async(&self) -> u32 {
    let mut example = NotSend::default();
    let local = "local".to_owned();
  
    example.something.insert("key1", "static".as_value());
    example.something.insert("key2", local.as_value());
  
    println!("{example:?}");

    let result = self.do_async().await;

    example.something.insert("key3", "another".as_value());  
    println!("{example:?}");

    result
}

Let's say I implement Send for this structure and try to move it into task:

#[tokio::main]
async fn main() {
    let mut example = NotSend::default();
    let local = "local".to_owned();

    example.something.insert("key1", "static".as_value());
    example.something.insert("key2", local.as_value());

    tokio::spawn(async move {
        println!("{example:?}");
    });

    tokio::time::sleep(Duration::from_secs(1)).await;
}

Here I will have error since variable "local" does not live long enough which prevents sending NotSend to another task and it looks exactly what I need.
But I'm not sure what are cases when NotSend with implemented Send trait will fail to work properly.

[alice]

My suggestion is that you don't do this. Instead, create your own type that doesn't use the valuable type, and which has no lifetime annotations. If you need the valuable type at some point so you can give it to a library, then convert your custom type into the valuable type then, and not before.

[SkiFire13]

Have you considered using Box<dyn Valuable + Send> instead of Value?

[nduchenk]

It should help, but I do not want to do additional allocation wrapping values into Box.

[nduchenk]

This is what I've ended up with:

#[derive(Default)]
struct NotSend<'a> {
    something: HashMap<&'a str, Value<'a>>,
}

type Value<'a> = &'a (dyn Valuable + Send + Sync);

Saving references to the values which implement traits I need for everything to work, but with limitations that I won't be able to put there reference to something like Rc.