From 10f0155491e81e317edcd74083994463cee76d50 Mon Sep 17 00:00:00 2001 From: Jared Moulton Date: Thu, 21 Nov 2024 11:03:33 -0700 Subject: [PATCH] make signals Not Send (#691) --- examples/counter-simple/src/main.rs | 34 ++++++---------- reactive/src/id.rs | 13 ++++-- src/ext_event.rs | 61 ++++++++++++++++++++++++----- 3 files changed, 73 insertions(+), 35 deletions(-) diff --git a/examples/counter-simple/src/main.rs b/examples/counter-simple/src/main.rs index c32a6181..aa50af29 100644 --- a/examples/counter-simple/src/main.rs +++ b/examples/counter-simple/src/main.rs @@ -1,26 +1,16 @@ -use floem::{ - reactive::create_signal, - views::{button, label, Decorators}, - IntoView, -}; +use floem::prelude::*; -fn app_view() -> impl IntoView { - // Create a reactive signal with a counter value, defaulting to 0 - let (counter, mut set_counter) = create_signal(0); - - // Create a vertical layout - ( - // The counter value updates automatically, thanks to reactivity - label(move || format!("Value: {counter}")), - // Create a horizontal layout - ( - button("Increment").action(move || set_counter += 1), - button("Decrement").action(move || set_counter -= 1), - ), - ) - .style(|s| s.flex_col()) +fn main() { + floem::launch(counter_view); } -fn main() { - floem::launch(app_view); +fn counter_view() -> impl IntoView { + let mut counter = RwSignal::new(0); + + h_stack(( + button("Increment").action(move || counter += 1), + label(move || format!("Value: {counter}")), + button("Decrement").action(move || counter -= 1), + )) + .style(|s| s.size_full().items_center().justify_center().gap(10)) } diff --git a/reactive/src/id.rs b/reactive/src/id.rs index 36043dea..3b02759a 100644 --- a/reactive/src/id.rs +++ b/reactive/src/id.rs @@ -1,16 +1,23 @@ -use std::sync::atomic::AtomicU64; +use std::{marker::PhantomData, sync::atomic::AtomicU64}; use crate::{effect::observer_clean_up, runtime::RUNTIME, signal::Signal}; +/// Marker type explaining why something can't be sent across threads +#[allow(dead_code)] +struct NotThreadSafe(*const ()); + /// An internal id which can reference a Signal/Effect/Scope. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Hash)] -pub struct Id(u64); +pub struct Id(u64, PhantomData); impl Id { /// Create a new Id that's next in order pub(crate) fn next() -> Id { static COUNTER: AtomicU64 = AtomicU64::new(0); - Id(COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed)) + Id( + COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + PhantomData, + ) } /// Try to get the Signal that links with this Id diff --git a/src/ext_event.rs b/src/ext_event.rs index fefad07a..64c5f166 100644 --- a/src/ext_event.rs +++ b/src/ext_event.rs @@ -1,8 +1,8 @@ use std::{cell::Cell, collections::VecDeque, sync::Arc}; use floem_reactive::{ - create_effect, untrack, with_scope, ReadSignal, Scope, SignalGet, SignalUpdate, Trigger, - WriteSignal, + create_effect, create_rw_signal, untrack, with_scope, ReadSignal, RwSignal, Scope, SignalGet, + SignalUpdate, SignalWith, WriteSignal, }; use parking_lot::Mutex; @@ -12,10 +12,51 @@ use crate::{ Application, }; +#[derive(Debug)] +/// # SAFETY +/// +/// **DO NOT USE THIS** trigger except for when using with `create_ext_action` or when you guarentee that +/// the signal is never used from a different thread than it was created on. +pub struct ExtSendTrigger { + signal: RwSignal<()>, +} + +impl Copy for ExtSendTrigger {} + +impl Clone for ExtSendTrigger { + fn clone(&self) -> Self { + *self + } +} + +impl ExtSendTrigger { + pub fn notify(&self) { + self.signal.set(()); + } + + pub fn track(&self) { + self.signal.with(|_| {}); + } + + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + create_trigger() + } +} + +pub fn create_trigger() -> ExtSendTrigger { + ExtSendTrigger { + signal: create_rw_signal(()), + } +} + +unsafe impl Send for ExtSendTrigger {} +unsafe impl Sync for ExtSendTrigger {} + pub(crate) static EXT_EVENT_HANDLER: ExtEventHandler = ExtEventHandler::new(); pub(crate) struct ExtEventHandler { - pub(crate) queue: Mutex>, + pub(crate) queue: Mutex>, } impl Default for ExtEventHandler { @@ -31,7 +72,7 @@ impl ExtEventHandler { } } - pub fn add_trigger(&self, trigger: Trigger) { + pub fn add_trigger(&self, trigger: ExtSendTrigger) { { // Run this in a short block to prevent any deadlock if running the trigger effects // causes another trigger to be registered @@ -43,7 +84,7 @@ impl ExtEventHandler { } } -pub fn register_ext_trigger(trigger: Trigger) { +pub fn register_ext_trigger(trigger: ExtSendTrigger) { EXT_EVENT_HANDLER.add_trigger(trigger); } @@ -53,7 +94,7 @@ pub fn create_ext_action( ) -> impl FnOnce(T) { let view = get_current_view(); let cx = cx.create_child(); - let trigger = cx.create_trigger(); + let trigger = with_scope(cx, ExtSendTrigger::new); let data = Arc::new(Mutex::new(None)); { @@ -87,7 +128,7 @@ pub fn update_signal_from_channel( rx: crossbeam_channel::Receiver, ) { let cx = Scope::new(); - let trigger = cx.create_trigger(); + let trigger = with_scope(cx, ExtSendTrigger::new); let channel_closed = cx.create_rw_signal(false); let data = Arc::new(Mutex::new(VecDeque::new())); @@ -123,7 +164,7 @@ pub fn create_signal_from_channel( rx: crossbeam_channel::Receiver, ) -> ReadSignal> { let cx = Scope::new(); - let trigger = cx.create_trigger(); + let trigger = with_scope(cx, ExtSendTrigger::new); let channel_closed = cx.create_rw_signal(false); let (read, write) = cx.create_signal(None); @@ -211,14 +252,14 @@ pub fn create_signal_from_stream( use futures::task::{waker, ArcWake}; let cx = Scope::current().create_child(); - let trigger = cx.create_trigger(); + let trigger = with_scope(cx, ExtSendTrigger::new); let (read, write) = cx.create_signal(initial_value); /// Waker that wakes by registering a trigger // TODO: since the trigger is just a `u64`, it could theoretically be changed to be a `usize`, // Then the implementation of the std::task::RawWakerVTable could pass the `usize` as the data pointer, // avoiding any allocation/reference counting - struct TriggerWake(Trigger); + struct TriggerWake(ExtSendTrigger); impl ArcWake for TriggerWake { fn wake_by_ref(arc_self: &Arc) { EXT_EVENT_HANDLER.add_trigger(arc_self.0);