dioxus icon indicating copy to clipboard operation
dioxus copied to clipboard

A general issue with signals "Already Borrowed"

Open patrik-cihal opened this issue 7 months ago • 6 comments

Problem

Signals are extremely unstable. For me they keep leading to panics in production on the client side.

For example:

                        on_delete_point: move |i: usize| async move {
                            let point = points.write().remove(i);
                            point_fetched_all.write().remove(i);
                            markets.write().remove(i);
                            delete_point(point.id).await.expect("Should be able to delete point");

                            user_actions_context.write().push(format!("Deleted point {:?}", point.text_embedding));
                        },

If I call this twice, while point is already being deleted: Panic.

I don't thing signals are a good model and am not sure if there is even a way to address this without getting rid of them. But man. It's painful.

I've already said this in Dioxus's Discord. I really like Dioxus and think that the way you're doing most things are very smart. I am still not convinced that state management is one of them.

Environment:

  • Dioxus version: 0.6.3

patrik-cihal avatar May 15 '25 17:05 patrik-cihal

I suspect the main cause of this is holding locks over await points. If you don't hold locks over await points and you don't use the unchecked read or write methods, the borrow checker should prevent all borrowing issues.

Clippy could help here if we added our locks to the list of lints. Eg. Clippy will warn about the four different lifetime issues in this code if you define this clippy.toml:

  • clippy.toml
await-holding-invalid-types = [
  "generational_box::GenerationalRef",
  { path = "generational_box::GenerationalRef", reason = "Reads should not be held over an await point" },
  "generational_box::GenerationalRefMut",
  { path = "generational_box::GenerationalRefMut", reason = "Write should not be held over an await point" },
  "dioxus_signals::Write",
  { path = "dioxus_signals::Write", reason = "Write should not be held over an await point" },
]
  • main.rs
use dioxus::prelude::*;

fn main() {
    launch(|| {
        let mut state = use_signal(|| 0);
        let mut boxed = use_hook(|| CopyValue::new(0));
        rsx! {
            button {
                onclick: move |_| async move {
                    let read = state.read();
                    other(&read).await;
                }
            }

            button {
                onclick: move |_| async move {
                    let mut write = state.write();
                    other_mut(&mut write).await;
                }
            }

            button {
                onclick: move |_| async move {
                    let read = boxed.read();
                    other(&read).await;
                }
            }

            button {
                onclick: move |_| async move {
                    let mut write = boxed.write();
                    other_mut(&mut write).await;
                }
            }
        }
    })
}

async fn other(value: &u32) {
    println!("value: {value}");
}

async fn other_mut(value: &mut u32) {
    *value += 1;
}

ealmloff avatar May 15 '25 18:05 ealmloff

Hmm not everyone uses clippy. An approach which makes it enforced by the compiler would definitely be better.

The thing many people really like about Rust is that compiling implies very little chance of runtime panics.

patrik-cihal avatar May 15 '25 18:05 patrik-cihal

Encounter the same issue while updating SyncSignal in another thread.

iamcco avatar May 16 '25 01:05 iamcco

await-holding-invalid-types = [ "generational_box::GenerationalRef", { path = "generational_box::GenerationalRef", reason = "Reads should not be held over an await point" }, "generational_box::GenerationalRefMut", { path = "generational_box::GenerationalRefMut", reason = "Write should not be held over an await point" }, "dioxus_signals::Write", { path = "dioxus_signals::Write", reason = "Write should not be held over an await point" }, ]

Anyway this is pretty cool, and yes I was holding a locks over await points.

patrik-cihal avatar May 16 '25 07:05 patrik-cihal

I can also confirm that it didn't fix the issue.

patrik-cihal avatar May 16 '25 07:05 patrik-cihal

I haven't seen a viable alternative to signals that solves the same problems. Signals let you rerun only small parts of your application at a time without explicitly listing dependencies. The interior mutability in signals lets you read and write to the signal inside async blocks. Here is an async counter app that shows those cases:

use dioxus::prelude::*;

fn main() {
    dioxus::launch(App);
}

#[component]
pub fn App() -> Element {
    let mut some_other_state = use_signal(|| 0);
    let mut count = use_signal(|| 0);
    let half_memo = use_memo(move || {
        // Notice this only reruns when count changes, not when some_other_state changes
        println!("Reran half_memo");
        count / 2
    });
    let doubled_async = use_resource(move || async move {
        // This will rerun when half_memo changes, but not when count or some_other_state changes
        println!("Reran doubled_async");
        tokio::time::sleep(std::time::Duration::from_secs(2)).await;
        // You can freely read and modify state inside an async block as long just like you would with
        // any normal interior mutable state (Rc<RefCell<T>>)
        half_memo() * 2
    });

    println!("Reran App");

    rsx! {
        button {
            onclick: move |_| {
                some_other_state += 1;
            },
            "change some_other_state"
        }
        "some_other_state: {some_other_state}"
        button {
            onclick: move |_| {
                count += 1;
            },
            "Increment count"
        }
        div {
            "Half memo: {half_memo}"
        }
        div {
            if let Some(doubled) = doubled_async() {
                "(count / 2) * 2: {doubled}"
            } else {
                "Loading..."
            }
        }
    }
}

ealmloff avatar May 22 '25 15:05 ealmloff