Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot send RedisFuture between thread #156

Closed
boxdot opened this issue Jun 27, 2018 · 3 comments
Closed

Cannot send RedisFuture between thread #156

boxdot opened this issue Jun 27, 2018 · 3 comments

Comments

@boxdot
Copy link

boxdot commented Jun 27, 2018

This minimal example tries to execute a Redis command on a thread-pool (using the latest master)

extern crate redis;
extern crate futures;
extern crate tokio_threadpool;

use futures::Future;
use redis::Client;
use tokio_threadpool::ThreadPool;

fn main() {
    let client = Client::open("redis://localhost").unwrap();

    let work = client.get_async_connection().and_then(|conn| {
        let mut cmd = redis::cmd("SET");
        cmd.arg("x").arg(42);
        cmd.query_async(conn).and_then(|_| Ok(()))
    }).map_err(|_| ());

    let thread_pool = ThreadPool::new();
    thread_pool.spawn(work);
}

It does not compile due to:

the trait bound `futures::Future<Item=redis::async::Connection, Error=redis::RedisError>: std::marker::Send` is not satisfied
  --> src/main.rs:19:17
   |
19 |     thread_pool.spawn(work);
   |                 ^^^^^ `futures::Future<Item=redis::async::Connection, Error=redis::RedisError>` cannot be sent between threads safely
   |
   = help: the trait `std::marker::Send` is not implemented for `futures::Future<Item=redis::async::Connection, Error=redis::RedisError>`
   = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<futures::Future<Item=redis::async::Connection, Error=redis::RedisError>>`
   = note: required because it appears within the type `std::boxed::Box<futures::Future<Item=redis::async::Connection, Error=redis::RedisError>>`
   = note: required because it appears within the type `futures::future::chain::Chain<std::boxed::Box<futures::Future<Item=redis::async::Connection, Error=redis::RedisError>>, futures::AndThen<std::boxed::Box<futures::Future<Item=(redis::async::Connection, _), Error=redis::RedisError>>, std::result::Result<(), redis::RedisError>, [closure@src/main.rs:15:40: 15:50]>, [closure@src/main.rs:12:55: 16:6]>`
   = note: required because it appears within the type `futures::AndThen<std::boxed::Box<futures::Future<Item=redis::async::Connection, Error=redis::RedisError>>, futures::AndThen<std::boxed::Box<futures::Future<Item=(redis::async::Connection, _), Error=redis::RedisError>>, std::result::Result<(), redis::RedisError>, [closure@src/main.rs:15:40: 15:50]>, [closure@src/main.rs:12:55: 16:6]>`
   = note: required because it appears within the type `futures::MapErr<futures::AndThen<std::boxed::Box<futures::Future<Item=redis::async::Connection, Error=redis::RedisError>>, futures::AndThen<std::boxed::Box<futures::Future<Item=(redis::async::Connection, _), Error=redis::RedisError>>, std::result::Result<(), redis::RedisError>, [closure@src/main.rs:15:40: 15:50]>, [closure@src/main.rs:12:55: 16:6]>, [closure@src/main.rs:16:16: 16:22]>`

If I am not mistaken, this does not work, since RedisFuture is just a type alias to Box<Future<Item = T, Error = RedisError>> (note: there is no + Send in the box). If I add the annotation, then async::Connection::read_response does not compile. I did not follow the whole path, it looks like it boxes some types containing io::Read which is not Send-able.

I would love to look into this issue, but most likely I will need some support how to attack the problem. :)

@badboy
Copy link
Collaborator

badboy commented Jun 27, 2018

cc @Marwes - I don't know about the details (and have yet to use the async code). Is there any chance we can make Redis' future RedisFuture Send?

@Marwes
Copy link
Collaborator

Marwes commented Jun 27, 2018

It seems like I only fixed this in #143. Probably not worth extracting it but it should just mean changing this line https://github.com/mitsuhiko/redis-rs/pull/143/files#diff-dfd26227c4842bd3288e9b1f53f0089fL352 .

Edit: Nvm, easier to just change it.

Marwes pushed a commit to Marwes/redis-rs that referenced this issue Jun 27, 2018
Marwes pushed a commit to Marwes/redis-rs that referenced this issue Jun 27, 2018
@boxdot
Copy link
Author

boxdot commented Jun 27, 2018

@Marwes: Amazing. This is what I was missing. Needed to annotate T + other types as Send. Thanks!

boxdot pushed a commit to boxdot/redis-rs that referenced this issue Jun 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants