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

Missing Send in SessionData::discard's Future #580

Closed
jakubfijalkowski opened this issue Nov 17, 2021 · 3 comments · Fixed by #581
Closed

Missing Send in SessionData::discard's Future #580

jakubfijalkowski opened this issue Nov 17, 2021 · 3 comments · Fixed by #581
Labels

Comments

@jakubfijalkowski
Copy link

Hey there!

I stumbled on a following error. Consider the handler code:

pub async fn logout_handler(mut state: State) -> HandlerResult {
    let session = SessionData::<UserSession>::take_from(&mut state);
    session.discard(&mut state).await.unwrap();

    let response = create_empty_response(&state, StatusCode::OK);
    Ok((state, response))
}

attached to router using to_async. Nothing really fancy - just dropping the user state on logout. Before v0.6.1, the handler was synchronous, as discard returned plain Result. From 0.6.1 onward, discard returns a Pin<Box<dyn Future<Output = Result<(), SessionError>>>>. This is problematic, because the Future is not Send, so the future returned by logout_handler is also !Send and cannot be used in to_async.

Changing the signature of discard to

pub fn discard(
    self,
    state: &mut State,
) -> Pin<Box<dyn Future<Output = Result<(), SessionError>> + Send>> {

i.e. making the Future also Send works and solves the problem. I am not sure whether this is the correct approach - do I miss something here? I'm only starting with async Rust and I don't really feel like I know how it works, so sorry in advance if I missed something obvious. :)

@msrd0
Copy link
Member

msrd0 commented Nov 18, 2021

This looks like a problem in Gotham, likely introduced in #468

@msrd0 msrd0 added the bug label Nov 18, 2021
@jakubfijalkowski
Copy link
Author

Yep, it also looks like a simple omission (since other Futures are aliased, and the alias has Send). Do you want a PR from me maybe (I can also write some tests for discard, as noted in the comment), or do you want to tackle it yourself?

@msrd0
Copy link
Member

msrd0 commented Nov 18, 2021

I just created a PR myself, but thanks for the offer

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

Successfully merging a pull request may close this issue.

2 participants