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

Failure when using with axum-login #580

Closed
j0lol opened this issue Feb 26, 2023 · 4 comments
Closed

Failure when using with axum-login #580

j0lol opened this issue Feb 26, 2023 · 4 comments

Comments

@j0lol
Copy link
Contributor

j0lol commented Feb 26, 2023

My project uses axum-login for authentication, but to do this I need to pass axum_login::extractors::AuthContext around all server functions for authentication. To do this, I need to make custom handlers:

    async fn server_fn_handler(auth_context: AuthContext, path: Path<String>, headers: HeaderMap, request: Request<AxumBody>){
        handle_server_fns_with_context(path, headers, move |cx| {
            provide_context(cx, auth_context.clone());
        }, request).await;
    }

    async fn leptos_routes_handler(auth_context: AuthContext, Extension(options): Extension<Arc<LeptosOptions>>, req: Request<AxumBody>) -> Response{
            let handler = leptos_axum::render_app_to_stream_with_context((*options).clone(),
            move |cx| {
                provide_context(cx, auth_context.clone())
            },
            |cx| view! { cx, <TodoApp/> }
        );
        handler(req).await.into_response()
    }

and handle all routes with them:

        .route("/api/*fn_name", post(server_fn_handler))
        .leptos_routes_with_handler(routes, get(leptos_routes_handler) )

However, whenever I serve and load a page, my browser will fail to load the page, with this error in the cli:

thread 'tokio-runtime-worker' panicked at 'Session handle still has owners.: RwLock { mr: 536870911, s: Semaphore { permits: 536870911 }, c: UnsafeCell { .. } }', /home/jo/.cargo/registry/src/github.com-1ecc6299db9ec823/axum-sessions-0.4.1/src/session.rs:338:49

Unsure what's happening here, is leptos improperly holding onto something when it shouldn't?

@gbj
Copy link
Collaborator

gbj commented Feb 26, 2023

So the panic is coming from axum-sessions here: https://github.com/maxcountryman/axum-sessions/blob/4c7480a205953abbc89bbef56391af2ac94533af/src/session.rs#L346-L348

It looks like they are implementing Clone using an Arc, but assuming that there is actually only one clone of something in existence. I'm pretty surprised to see something like this in a Rust library, honestly.

This is pretty incompatible with the way context works in Leptos: when you provide_context the framework holds onto a copy of something, and when you use_context it clones it.

I'm going to close this because I don't think it really has anything to do with Leptos, but with the way they've chosen to implement this library.

@maxcountryman
Copy link

I'm pretty surprised to see something like this in a Rust library, honestly.

I'm not sure what's so surprising about interior mutability, however I do think there are ergonomic problems with this API. There is a reason for this however, so for future readers who stumble upon this: the underlying crate at issue here is async-session which has made the unfortunate choice to implement Clone such that it destroys the cookie value when clone is called. There aren't many great ways to work around this since the library authors haven't provided an escape hatch. Thus we're leveraging interior mutability here to avoid calling clone directly (because again that destroys the cookie value). It's actually not such an uncommon pattern in Rust, but the issue here is that it might be surprising if you aren't aware you need to manage the lock (which seems to be the issue in this case).

@gbj
Copy link
Collaborator

gbj commented Feb 26, 2023

I'm not sure what's so surprising about interior mutability
What I specifically mean is surprising here is providing a type that implements Clone that panics on use if it has in fact been cloned, without documenting that it panics, as far as I can tell.

The pattern I would've expected here, given what you're describing, is something like Arc<RwLock<Option<_>>> using .write() to get a write lock and .take() to take the value out of the Option, rather than asserting that the Arc only has one living reference (in which case, what's the point of the Arc...?)

the issue here is that it might be surprising if you aren't aware you need to manage the lock (which seems to be the issue in this case).

It might be useful to document how this is supposed to be done... I can't find any discussion of managing a lock anywhere in the axum-login or axum-session docs.

@maxcountryman
Copy link

Please feel free to open an issue or better yet a PR.

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