leptos-rs / leptos

Build fast web applications with Rust.
https://leptos.dev
MIT License
16.35k stars 655 forks source link

Multiple ReadGuards lead to deadlock #3158

Closed stefnotch closed 3 weeks ago

stefnotch commented 3 weeks ago

Describe the bug I cannot call .read more than once without causing reactive_graph (0.1.0-gamma3) to hang

Leptos Dependencies

any_spawner = { version = "0.1.1", features = ["futures-executor"] }
reactive_graph = { version = "0.1.0-gamma3", features = ["effects"] }

To Reproduce

use any_spawner::Executor;
use reactive_graph::{computed::Memo, effect::Effect, owner::Owner, traits::Read};

#[test]
fn test_memo_computed() {
    let _ = Executor::init_futures_executor();
    let _owner = Owner::new();
    let memo = Memo::<i32>::new_computed(|_| 42);

    Effect::new(move |_| {
        let guard_a = memo.read();
        let guard_b = memo.read();
        assert_eq!(guard_a, 42);
        assert_eq!(guard_b, 42);
    });
    Executor::poll_local();
}

Expected behavior I expected it to be possible to acquire multiple read guards. I also expect Leptos to panic when trying to get a write guard while a read guard is alive.

gbj commented 3 weeks ago

So just to start off: On a memo, taking one read lock and reusing it will always be better than taking multiple read locks, on a memo.

This is because .read() doesn't simply take a read-lock on the inner value: it has to run the reactive tracking logic as well. The tracking logic, not the current value, is what's causing the issue here. Specifically, calling .read() on the memo needs to be able to update the memo's current state.

3160 should fix this specific issue, I think (at least it does in the test cases I tried), but in general taking one read lock is strictly better.

I also expect Leptos to panic when trying to get a write guard while a read guard is alive.

This library does not add any special behavior to what Rust's std::sync::RwLock does in this scenario, which is apparently to deadlock.

stefnotch commented 3 weeks ago

Thank you for the quick fix! And yes, of course only having one guard is smarter. I just ran into it while doing some very nested function calls, and realising that something was causing my app to deadlock.