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

option for Send-able Session #251

Open
Oyelowo opened this issue May 29, 2022 · 2 comments
Open

option for Send-able Session #251

Oyelowo opened this issue May 29, 2022 · 2 comments
Labels
A-session Project: actix-session C-feature Category: new functionality

Comments

@Oyelowo
Copy link

Oyelowo commented May 29, 2022

Original title: std::marker::Send is not implemented for Rc<RefCell<actix_session::session::SessionInner>>

Expected Behavior

I am trying to use session object with Redis as the storage in a distributed system with async-graphql to set and delete session for userid but having issues with that because actix-session does not implement Send and cannot be moved across threads. It has type: Rc<RefCell<actix_session::SessionInner>>. async-graphql requires that it has a Send trait.

Current Behavior

Full compiler error message:

`Rc<RefCell<actix_session::session::SessionInner>>` cannot be sent between threads safely
within `actix_session::Session`, the trait `std::marker::Send` is not implemented for `Rc<RefCell<actix_session::session::SessionInner>>`rustc[E0277](https://doc.rust-lang.org/error-index.html#E0277)
graphql.rs(74, 23): required by a bound introduced by this call
lib.rs(1, 1): required because it appears within the type `actix_session::Session`
request.rs(91, 26): required by a bound in `Request::data`
`Rc<RefCell<actix_session::session::SessionInner>>` cannot be shared between threads safely
within `actix_session::Session`, the trait `Sync` is not implemented for `Rc<RefCell<actix_session::session::SessionInner>>`rustc[E0277](https://doc.rust-lang.org/error-index.html#E0277)
graphql.rs(74, 23): required by a bound introduced by this call
lib.rs(1, 1): required because it appears within the type `actix_session::Session`

Possible Solution

Make Session type an Arc/Mutex rather than a RefCell

Present workaround

use std::ops::Deref;

use actix_session::Session;
use send_wrapper::SendWrapper;
use uuid::Uuid;
use wither::bson::oid::ObjectId;
#[derive(Clone, Debug)]
struct Shared<T>(pub Option<SendWrapper<T>>);

impl<T> Shared<T> {
    pub fn new(v: T) -> Self {
        Self(Some(SendWrapper::new(v)))
    }
}

impl<T> Deref for Shared<T> {
    type Target = T;

    fn deref(&self) -> &Self::Target {
        &*self.0.as_deref().unwrap()
    }
}

type SessionShared = Shared<actix_session::Session>;

#[derive(Debug, thiserror::Error)]
pub enum TypedSessionError {
    #[error("Failed to parse data")]
    ParsingFailure(#[from] serde_json::Error),

    #[error(transparent)]
    Unknown(#[from] anyhow::Error), // source and Display delegate to anyhow::Error
}

type TypedSessionResult<T> = Result<T, TypedSessionError>;

/*
This is somewhat like a hack: https://github.com/async-graphql/async-graphql/issues/426
Session is  Session(Rc<RefCell<SessionInner>>) and probably okay to use
SendWrapper for now but having it implement Send would allow
using Arc/Mutex
*/
pub struct TypedSession(SessionShared);

impl TypedSession {
    const USER_ID_KEY: &'static str = "user_id";

    pub fn new(session: Session) -> Self {
        Self(Shared::new(session))
    }

    pub fn renew(&self) {
        self.0.renew();
    }

    pub fn insert_user_uuid(&self, user_id: Uuid) -> TypedSessionResult<()> {
        self.0.insert(Self::USER_ID_KEY, user_id)?;
        Ok(())
    }

    pub fn get_user_uuid(&self) -> TypedSessionResult<Option<Uuid>> {
        Ok(self.0.get::<Uuid>(Self::USER_ID_KEY)?)
    }

    pub fn clear(&self) {
        self.0.clear()
    }
}


## Steps to Reproduce (for bugs)
Try with async-graphql `GraphqlRequest` as shown below

```rs
#[post("/graphql")]
pub async fn index(
    schema: web::Data<MyGraphQLSchema>,
    req: HttpRequest,
    gql_request: GraphQLRequest,
) -> GraphQLResponse {
    let mut request = gql_request.into_inner();
    let session = req.get_session();
    request = request.data(session); // ---> This will fail because it expects it to have a Send Marker

    schema.execute(request).await.into()
}

Your Environment

Mac OS monterey macbook m1 max 14 inch 2021

  • Rust Version (I.e, output of rustc -V): Rust 1.61
  • Actix-* crate(s) Version: actix-session: 0.6.2

Sidenote:
Issues are also mentioned here:

@robjtede robjtede changed the title std::marker::Send is not implemented for Rc<RefCell<actix_session::session::SessionInner>> std::marker::Send is not implemented for Rc<RefCell<actix_session::session::SessionInner>> May 30, 2022
@robjtede robjtede added A-session Project: actix-session C-feature Category: new functionality labels Jul 19, 2022
@robjtede robjtede changed the title std::marker::Send is not implemented for Rc<RefCell<actix_session::session::SessionInner>> option for Send-able Session Jul 19, 2022
@robjtede
Copy link
Member

robjtede commented Jul 19, 2022

The Rc<RefCell<T>> wrapper is an intentional choice to take advantage of Actix Web's ability to support !Send handler but I understand other parts of the ecosystem have trouble with !Send types such as this.

What I'm thinking for the solution is an opt-in crate feature that compiles Session as Arc<RwLock<actix_session::SessionInner>>:

actix-session = { version = "...", features = ["send"] }

Feature name to be bikeshed.

Thoughts from @actix/contributors?

@LukeMathWalker
Copy link
Contributor

LukeMathWalker commented Jul 19, 2022

I'd be somewhat wary of making that kind of change via a cargo feature. What happens if you end up with two different subsets of your dependency tree needing the opposite behaviour?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-session Project: actix-session C-feature Category: new functionality
Projects
None yet
Development

No branches or pull requests

3 participants