Parallel access of signals panics

[QuartzLibrary]

If we spawn threads and get/set an ArcRwSignal, it panics:

thread '<unnamed>' panicked at /home/user/code/leptos/reactive_graph/src/traits.rs:388:39:
At reactive_graph/tests/signal.rs:156:28, you tried to access a reactive value which was defined at reactive_graph/tests/signal.rs:149:18, but it has already been disposed.
stack backtrace:
   0: rust_begin_unwind
             at /rustc/e71f9a9a98b0faf423844bf0ba7438f29dc27d58/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/e71f9a9a98b0faf423844bf0ba7438f29dc27d58/library/core/src/panicking.rs:76:14
   2: core::panicking::panic_display
             at /home/user/.rustup/toolchains/1.84.1-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panicking.rs:269:5
   3: reactive_graph::traits::Get::get::{{closure}}::panic_cold_display
             at /home/user/.rustup/toolchains/1.84.1-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panic.rs:100:13
   4: reactive_graph::traits::Get::get::{{closure}}
             at ./src/traits.rs:75:17
   5: core::option::Option<T>::unwrap_or_else
             at /home/user/.rustup/toolchains/1.84.1-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:1017:21
   6: reactive_graph::traits::Get::get
             at ./src/traits.rs:388:9
   7: signal::threaded_chaos::{{closure}}::{{closure}}
             at ./tests/signal.rs:156:21

I believe the issue is in these lines where the code attempts to get the lock, but fails if there is contention:

https://github.com/leptos-rs/leptos/blob/main/reactive_graph/src/signal/arc_rw.rs#L251-L257

Not sure if this is the intended behavior. (Maybe to avoid the risk of deadlocks if the .read() API is used?)

If it is, I'll just fix the error message and document the test should panic, otherwise we can explore other solutions.

(This was found while testing #3650.)

[QuartzLibrary]

Looking more closely, currently:

• Read access panics if there is contention (an ongoing write).
• Write access waits on the lock (potentially deadlocking).

I added two smaller tests to demonstrate the behavior.

[QuartzLibrary]

Thinking through the options:

• The largest issue is around the .{read,write}() API, but I assume that is a given now since it would be a big breaking change to remove it.
• The read path could wait on the lock, avoiding the panics at the cost of deadlocking instead. There would be fewer unnecessary panics in multi-threaded code though.
• The impls for set and get can be overwritten so that they can't panic or deadlock on their own, to limit the issue.
• Copy-on-write would solve the issue, making both panics and deadlocks impossible. This would be at the cost of potentially unexpected semantics when writes overlap (editing two independent copies + last-write-wins, or similar) and a performance penalty. The weird semantics could be traded for deadlocks or panics only on writes.
• ...?

I'll let this sit for a bit to see if I can think of something better.

[gbj]

The intended behavior here for .read() and .write() is the behavior of std::sync::RwLock, which is what it's built on. To the extent that other parts of the implementation cause departures, I'm happy to see if we can fix them to bring that into alignment.

Of your test casts, I would expect read_during_write and overlapping_writes to fail, as written.

threaded_get_set is interesting. I would note that using .try_get() instead of .get() causes it to run without panicking, at the expense of something giving you None instead of the value if there is contention.

A blocking version should be possible by using .take() instead of .try_take() here, but it would be worth digging into the issue described in that change to make sure it did not reintroduce that behavior.

[QuartzLibrary]

That makes sense, users can always opt into more liberal cloning with extension traits.

I think the main intuitions that the current behavior breaks are:

• get and set never panic if just the two of them are used (no read or write).
  This can be fixed by specializing those implementations to always lock, at the cost of trading some panics for deadlocks in other places. What should be the overarching intuition here? try_ methods can fail on contention, and direct methods {deadlock,panic} on contention?
• Arc* values never panic on access (even try_get), because they are not part of the reactive scope handling (assuming no manual disposing).
  (I don't think this one is compatible anymore with the new read/write APIs.)

Happy to look into a preferred direction if you have one, clarify the error message, or just leave everything as-is and close.