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 keep ref to extension objects across .await points in handlers/middlewares #295

Closed
X-OrBit opened this issue Mar 25, 2024 · 3 comments · Fixed by #309
Closed

Cannot keep ref to extension objects across .await points in handlers/middlewares #295

X-OrBit opened this issue Mar 25, 2024 · 3 comments · Fixed by #309
Assignees
Labels
bug Something isn't working

Comments

@X-OrBit
Copy link

X-OrBit commented Mar 25, 2024

Describe the bug
Cannot use socket extensions inside async namespace middleware

To Reproduce
Code that reproduce error:

main.rs

use socketioxide::extensions::{Extensions, Ref};
use socketioxide::extract::SocketRef;
use socketioxide::handler::ConnectHandler;
use socketioxide::SocketIo;
use tokio::net::TcpListener;

fn handler(s: SocketRef) {
    println!("socket connected on / namespace with id: {}", s.id);
}

async fn test_async() -> Result<(), reqwest::Error> {
    reqwest::get("https://google.com").await?;
    Ok(())
}

async fn middleware(socket: SocketRef) -> Result<(), socketioxide::SocketError<()>> {
    let ext: &Extensions = &socket.extensions;
    let number: Option<Ref<i32>> = ext.get::<i32>(); // if comment this line
    let _ = test_async().await;                      // or this one, then there will be no compilation error 
    Ok(())
}

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let (layer, io) = SocketIo::new_layer();
    io.ns("/", handler.with(middleware));
    let app = axum::Router::new().layer(layer);

    let listener = TcpListener::bind("127.0.0.1:5002").await.unwrap();

    axum::serve(listener, app).await?;

    Ok(())
}

Cargo.toml

[package]
name = "test"
version = "0.1.0"
edition = "2021"

[dependencies]
axum = { version = "0.7.5" }
socketioxide = { version = "0.12.0", features = ["extensions"] }
reqwest = "0.12.2"

Error:

error[E0277]: the trait bound `fn(SocketRef) -> impl Future<Output = Result<(), SocketError<()>>> {middleware}: ConnectMiddleware<LocalAdapter, _>` is not satisfied
   --> src/bin/test_socket.rs:25:24
    |
25  |     io.ns("/", handler.with(middleware));
    |                        ^^^^ the trait `ConnectMiddleware<LocalAdapter, _>` is not implemented for fn item `fn(SocketRef) -> impl Future<Output = Result<(), SocketError<()>>> {middleware}`
    |
note: required by a bound in `ConnectHandler::{opaque#0}`
   --> /Users/xorbit/.cargo/registry/src/index.crates.io-6f17d22bba15001f/socketioxide-0.12.0/src/handler/connect.rs:239:12
    |
239 |         M: ConnectMiddleware<A, T1> + Send + Sync + 'static,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ConnectHandler::{opaque#0}`

Expected behavior
The code is being compiled, extensions are working properly

Versions (please complete the following information):

  • Socketioxide version: 0.12.0
  • Http lib: axum/0.7.5

Additional context
-

@X-OrBit X-OrBit added the bug Something isn't working label Mar 25, 2024
@Totodore
Copy link
Owner

Totodore commented Apr 3, 2024

It is because the Ref is not Send. Therefore if you use it across .await points, the returned future will not be Send. It is a requirement for the returned future to be Send so it can be later boxed.

One possible way to solve this might be to change the Dashmap that is backing the Extensions by a tokio::RwLock<HashMap> so that LockGuard are Send.

Currently the only solution to solve your issue is either to constraint your ref to a limited scope before your .await point:

 
async fn middleware(socket: SocketRef) -> Result<(), socketioxide::SocketError<()>> {
    {
         let ext: &Extensions = &socket.extensions;
         let number: Option<Ref<i32>> = ext.get::<i32>();
    }
    let _ = test_async().await;
    Ok(())
}

or to clone the content and discard the Ref.

@X-OrBit
Copy link
Author

X-OrBit commented Apr 5, 2024

Thanks for the answer, I just tried to use cloning for now, as the objects where not too complicated. But the solution with the limitation of the scope is good one, did not figure out it myself

Are there any plans to add rwlock/mutex to extensions?

@Totodore Totodore changed the title Cannot use socket extensions inside async namespace middleware Cannot keep ref to extension object across .await points in handlers/middlewares Apr 15, 2024
@Totodore Totodore changed the title Cannot keep ref to extension object across .await points in handlers/middlewares Cannot keep ref to extension objects across .await points in handlers/middlewares Apr 15, 2024
@Totodore
Copy link
Owner

Totodore commented Apr 15, 2024

Actually, it is global to every handlers (not only middlewares).
The only thing I could do is to provide a Extension extractor that directly get and clone the object like with the Extension extractor from axum. I might also add an HttpExtension extractor that would correspond to the http request extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants