Futures Sync

[Dzordzu]

• Added Sync trait requirements
• Implemented Sync traits for some structs

Changes allow to use async_trait_with_sync (allowed returning Client and JoinHandle)

[sfackler]

What is the rationale for needing Sync futures?

[Dzordzu]

What is the rationale for needing Sync futures?

I'm writing a library, where I want to pass an async function as the argument to another async function. After a little struggle I've managed to get

    async fn handle_simple(
        &mut self,
        handler: &'static dyn (Fn(String) -> HandlerResult),
    ) 

That could not work, because of the

handler has type `&dyn Fn(std::string::String) -> HandlerResult` which is not `Send`, because `dyn Fn(std::string::String) -> HandlerResult` is not `Sync``

So the solution was to make Fn sync.

    async fn handle_simple(
        &mut self,
        handler: &'static (dyn (Fn(String) -> HandlerResult) + Send + Sync),
    ) {

And, as I was using tokio_postgres, I've encountered get_type_rec and prepare_rec. After a quick fix (creating this breaking changes), I've managed to use tokio-postgres from within passed function.

TLDR; When one is using tokio-postgres from async function passed to an async method

This is a breaking change.

Yep. It's just a simple (working) concept of what can it look like. I was wondering, if there can be flag, that will simply add Sync type restrictions. Unfortunately, type aliases are unstable feature, so it may require some boilerplate

[sfackler]

What type is HandlerResult? as far as I can tell, that is a blocking function passed into an asynchronous method.

You may want to use https://docs.rs/sync_wrapper/latest/sync_wrapper/struct.SyncWrapper.html to wrap the futures in something that implements Sync in your own code.

[Dzordzu]

What type is HandlerResult?

Oh. I forgot to include its declaration. It's a future

HandlerResult: Future<Output = (String,i8)> + Sync + Send

You may want to use https://docs.rs/sync_wrapper/latest/sync_wrapper/struct.SyncWrapper.html to wrap the futures in something that implements Sync in your own code.

I was already struggling with it, but I could not manage to use it in my case. Obviously I'm still looking for using it, instead of this PR

[Dzordzu]

Ok. I've got an idea. As of now, the changes for async require only a little replace in

• tokio-postgres/src/prepare.rs (Send + Sync)
• tokio-postgres/src/codec.rs (CopyData and Send + Sync)
• tokio-postgres/src/copy_in.rs (CopyData and Send + Sync)
• tokio-postgres/src/copy_out.rs (CopyData)

As it was previously said, the best option would be to make this feature a non breaking change. The easiest way of doing it may be using a codegen to copy tokio-postgres. Then in the selected (ex. via preceding comment) lines Send with Send + Sync and CopyData with CopyDataSync should be replaced . CopyDataSync is a new struct with impl introduced with my previous commit.

This will allow to create a separate crate, let's name tokio-postgres-sync. The maintenance of this solution will be minimal, as everything will be synced with the proper tokio-postgres crate.

@sfackler what do you think? Is this acceptable?

[sfackler]

That is a thing you could make, but I am not particularly interested in maintaining. Creating copies of every async library you want to use with -sync added to the end does not seem like a particularly sustainable approach, and IMO indicates that the need for Sync futures is something to avoid.

[Dzordzu]

Yey! I've finally managed to avoid unnecessary code duplication in this repo. The hack was (simply) to use wihin my code following type for the arg

#[async_trait]
pub trait SimpleHandling<HandlerResult>
where
    HandlerResult: Future<Output = (String,i8)> + Send
{
    async fn handle_simple(&mut self, handler: impl (Fn(String) -> HandlerResult) + Send + Sync);
}

That allowed to use non Sync futures. @sfackler sorry, for wasting your time. In the future I'll try to help with real contribution there.

Also, I'm very grateful for instructions, discussion and code reviews. Thanks!

EDIT: To allow passing fn across threads I had

#[async_trait]
pub trait SimpleHandling<HandlerResult>
where
    HandlerResult: Future<Output = (String,i8)> + Send
{
    async fn handle_simple(&mut self, handler: Arc<impl (Fn(String) -> HandlerResult) + Send + Sync)>;
}

[IoIxD]

This solution is horrible (especially the one the repo's owner linked) and actually led me to have to write unsafe code to do a bitwise copy so that i could use SyncWrapper's .into_inner() without rust's borrow checker complaining.

If anybody happens to stumble upon this, you should know that sqlx is an alternative and happens to implement Sync. This is not to downplay the library owner's work (especially since this library has its own upsides), but its certainly a better solution then using SyncWrapper.