Skip to content

Conversation

@xacrimon
Copy link

@xacrimon xacrimon commented Oct 25, 2025

This refactors state management in ArcAsyncDerived to reduce the amount of separate locks.
This also replaces the AsyncRwLock from the async_lock library with a normal rwlock.

@xacrimon xacrimon changed the title Replace async lock in reactive_graph Refactor ArcAsyncDerived to remove async_lock dependecy Oct 26, 2025
@xacrimon xacrimon changed the title Refactor ArcAsyncDerived to remove async_lock dependecy Refactor ArcAsyncDerived Oct 26, 2025
@xacrimon xacrimon changed the title Refactor ArcAsyncDerived WIP: Refactor ArcAsyncDerived Oct 26, 2025
@gbj
Copy link
Collaborator

gbj commented Oct 26, 2025

Here is an example of some code that currently compiles and works correctly, but no longer compiles with this PR:

#[component]
pub fn App() -> impl IntoView {
    #[derive(Debug, Deserialize, Serialize)]
    struct SomeData {
        value: i32,
        // imagine there are other fields here that are expensive to clone
    }
    // load the posts
    let resource1 = Resource::new(|| (), |_| async { SomeData { value: 13 } });
    let resource2 = Resource::new(|| (), |_| async { SomeData { value: 42 } });
    let view = move || {
        Suspend::new(async move {
            let value_a = resource1.by_ref();
            let value_b = resource2.by_ref();
            let (value_a, value_b) =
                futures::future::join(value_a, value_b).await;

            value_a.value + value_b.value
        })
    };
    view! { <Suspense>{view}</Suspense> }
}

Specifically, this is because ArcRwLockReadGuardian is !Send (as is plain RwLockReadGuard of course; this is not guardian-specific), whereas the current async_lock::ReadArc is Send + Sync. The locks-by-reference therefore can't be held across an .await without the Future that uses them becoming !Send, which is not compatible with the requirements of Leptos SSR rendering here.

Now that I've had a chance to look at it in a little more detail, I think this is probably the primary reason I used an async lock here in the first place: less so that it can be read from without blocking, and more so that the read guard can be held across an await without needing to clone the inner value.

@xacrimon
Copy link
Author

xacrimon commented Oct 26, 2025

Right. Yeah, that makes sense.
That said, this was a draft, the reason I asked the question about breaking changes is because I wanted to move wakers and loading into ArcAsyncDerivedInner since the multitude of locks are currently very hard to reason about, since what we often actually want is to control the interactions between multiple items, like loading and wakers in the future impls.

I tried to do this and ran into issues since it naturally means modifying ArcAsyncDerivedReadyFuture into holding an Arc<RwLock<ArcAsyncDerivedInner>>, however this future is somewhat oddly reused in leptos-server for OnceResource, and it's constructor is public, which means I'd have to break either reactive-graph, leptos-server or both for this. I reckon OnceResource could similarly benefit in cleanliness, type size and allocation reduction from the same treatment I want to give ArcAsyncDerived however.

@xacrimon
Copy link
Author

Here is an example of some code that currently compiles and works correctly, but no longer compiles with this PR:

#[component]
pub fn App() -> impl IntoView {
    #[derive(Debug, Deserialize, Serialize)]
    struct SomeData {
        value: i32,
        // imagine there are other fields here that are expensive to clone
    }
    // load the posts
    let resource1 = Resource::new(|| (), |_| async { SomeData { value: 13 } });
    let resource2 = Resource::new(|| (), |_| async { SomeData { value: 42 } });
    let view = move || {
        Suspend::new(async move {
            let value_a = resource1.by_ref();
            let value_b = resource2.by_ref();
            let (value_a, value_b) =
                futures::future::join(value_a, value_b).await;

            value_a.value + value_b.value
        })
    };
    view! { <Suspense>{view}</Suspense> }
}

Specifically, this is because ArcRwLockReadGuardian is !Send (as is plain RwLockReadGuard of course; this is not guardian-specific), whereas the current async_lock::ReadArc is Send + Sync. The locks-by-reference therefore can't be held across an .await without the Future that uses them becoming !Send, which is not compatible with the requirements of Leptos SSR rendering here.

Now that I've had a chance to look at it in a little more detail, I think this is probably the primary reason I used an async lock here in the first place: less so that it can be read from without blocking, and more so that the read guard can be held across an await without needing to clone the inner value.

Right, what do you think about keeping parking_lot instead then? It allows send guards and is a much more common dependency than async-lock.

@gbj
Copy link
Collaborator

gbj commented Oct 29, 2025

Sure... Looks like parking_lot has an ArcRwLockReadGuard that is Send + Sync + 'static, so that should work well as a replacement.

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

Successfully merging this pull request may close these issues.

2 participants