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

Async connection deadlocks with multiple runtime instances #295

Closed
nazar-pc opened this issue Mar 17, 2020 · 9 comments
Closed

Async connection deadlocks with multiple runtime instances #295

nazar-pc opened this issue Mar 17, 2020 · 9 comments

Comments

@nazar-pc
Copy link

@nazar-pc nazar-pc commented Mar 17, 2020

I'm suffering doing basic calls with asynchronous version of the client for more than an hour now.
The trick is that I'm creating a connection in one Actix runtime instance and then using it from a different instance in a different thread (potentially different runtime).
In that case simple calls simply get stuck, even though Redis definitely receives commands, but my thread is dead and execution doesn't go past .await point until app shutdown (at which point I can see either result or error, no idea why).

Having the same behavior with multiplexed async connection.

Here is a minimal test case:

#[cfg(test)]
mod tests {
    use redis::{AsyncCommands, Client, RedisError};
    use std::thread;
    use tokio::runtime::Runtime;

    #[test]
    fn test() {
        Runtime::new().unwrap().block_on(async {
            let client = Client::open("redis://127.0.0.1:6379/").unwrap();
            let mut connection = client.get_async_connection().await.unwrap();

            let jojn_handle = thread::spawn(move || {
                Runtime::new().unwrap().block_on(async {
                    let result: Result<u8, RedisError> = connection.zadd("x", "y", 1).await;
                    assert!(result.is_ok());
                });
            });
            jojn_handle.join().unwrap();
        });
    }
}

This test will never finish.

Any suggestions or fixes?

@Marwes

This comment has been minimized.

Copy link
Collaborator

@Marwes Marwes commented Mar 17, 2020

You are blocking the outer runtime when you join the thread. Since it won't make progress on other tasks (such as the driver task for the redis connection). The inner runtime will end up being blocked.

@nazar-pc

This comment has been minimized.

Copy link
Author

@nazar-pc nazar-pc commented Mar 17, 2020

Why does it matter if I'm awaiting inside of the inner one for the command to be executed?

@nazar-pc

This comment has been minimized.

Copy link
Author

@nazar-pc nazar-pc commented Mar 17, 2020

Also in my app (which is bigger than above test) outer runtime (actix_rt) is not blocked, yet inner runtime (also actix_rt) is still in a separate thread, so if the behavior is the same, there must be a problem somewhere else.

@Marwes

This comment has been minimized.

Copy link
Collaborator

@Marwes Marwes commented Mar 17, 2020

You aren't doing await though in the example jojn_handle.join().unwrap(); blocks the entire thread which in turn blocks the outer runtime and eventloop which drives the redis connection.

@nazar-pc

This comment has been minimized.

Copy link
Author

@nazar-pc nazar-pc commented Mar 17, 2020

Hm... maybe my understanding is incorrect, but:

  1. There is no event loop in Rust's Futures implementation, it is all polling-based
  2. I already did .await to get connection, since it is not tied to particular runtime (and it doesn't seem to) it should not have any additional futures spawned on runtime directly, hence shouldn't really block anything
@Marwes

This comment has been minimized.

Copy link
Collaborator

@Marwes Marwes commented Mar 17, 2020

The connection is tied to a runtime (https://github.com/tokio-rs/mio/ / https://github.com/tokio-rs/tokio). You can still use it from another runtime but only as long as the runtime that it is created still runs.

In this example it is unable to since the join call blocks the entire runtime.

@nazar-pc

This comment has been minimized.

Copy link
Author

@nazar-pc nazar-pc commented Mar 17, 2020

Well, this one doesn't block for 999 seconds. And it will only process result after... 999 seconds:

#[cfg(test)]
mod tests {
    use redis::{AsyncCommands, Client, RedisError};
    use std::thread;
    use tokio::runtime::Runtime;
    use tokio::time::Duration;

    #[test]
    fn test() {
        Runtime::new().unwrap().block_on(async {
            let client = Client::open("redis://127.0.0.1:6379/").unwrap();
            let mut connection = client.get_async_connection().await.unwrap();

            let jojn_handle = thread::spawn(move || {
                Runtime::new().unwrap().block_on(async {
                    let result: Result<u8, RedisError> = connection.zadd("x", "y", 1).await;
                    assert!(result.is_ok());
                });
            });

            tokio::time::delay_for(Duration::from_secs(999)).await;
            jojn_handle.join().unwrap();
        });
    }
}
@Marwes

This comment has been minimized.

Copy link
Collaborator

@Marwes Marwes commented Mar 17, 2020

tokio::time::delay_for(Duration::from_secs(999)).await;

Yields the current task which allows the outer runtime to run. If you add a prinln! in the inner Runtime you can see that zadd runs as expected.

After that it does take 999 seconds due to the delay_for but then it shutsdown as expected (lower the time to ~5 seconds to see it easily).

You may need to run the test with cargo test -- --nocapture to see the print.

@nazar-pc

This comment has been minimized.

Copy link
Author

@nazar-pc nazar-pc commented Mar 17, 2020

You are right.
Still it definitely didn't work for me for some reason.
I'll close for now and will reopen when/if I have a reproducible example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.