From 728a26a2cc5bead4312f86aee504d4fa2654fda1 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 19 Jan 2024 22:01:21 -0800 Subject: [PATCH] Fix borrowmut error in nested spawns --- packages/core/src/runtime.rs | 2 +- packages/core/src/tasks.rs | 54 ++++++++++--------- packages/dioxus/src/launch.rs | 5 ++ .../examples/axum-hello-world/src/main.rs | 1 + packages/fullstack/src/launch.rs | 1 - 5 files changed, 37 insertions(+), 26 deletions(-) diff --git a/packages/core/src/runtime.rs b/packages/core/src/runtime.rs index 764d54f57..1373736c7 100644 --- a/packages/core/src/runtime.rs +++ b/packages/core/src/runtime.rs @@ -27,7 +27,7 @@ pub struct Runtime { pub(crate) rendering: Cell, /// Tasks created with cx.spawn - pub(crate) tasks: RefCell>, + pub(crate) tasks: RefCell>>, /// Queued tasks that are waiting to be polled pub(crate) queued_tasks: Rc>>, diff --git a/packages/core/src/tasks.rs b/packages/core/src/tasks.rs index 09ef4d6d6..91590d93a 100644 --- a/packages/core/src/tasks.rs +++ b/packages/core/src/tasks.rs @@ -1,11 +1,11 @@ use crate::innerlude::{remove_future, spawn, Runtime}; use crate::ScopeId; use futures_util::task::ArcWake; -use std::cell::RefCell; use std::future::Future; use std::pin::Pin; use std::sync::Arc; use std::task::Waker; +use std::{cell::RefCell, rc::Rc}; /// A task's unique identifier. /// @@ -47,21 +47,32 @@ impl Runtime { /// Spawning a future onto the root scope will cause it to be dropped when the root component is dropped - which /// will only occur when the VirtualDom itself has been dropped. pub fn spawn(&self, scope: ScopeId, task: impl Future + 'static) -> Task { - let mut tasks = self.tasks.borrow_mut(); + // Insert the task, temporarily holding a borrow on the tasks map + let (task, task_id) = { + let mut tasks = self.tasks.borrow_mut(); - let entry = tasks.vacant_entry(); - let task_id = Task(entry.key()); + let entry = tasks.vacant_entry(); + let task_id = Task(entry.key()); - let task = LocalTask { - parent: self.current_task(), - task: RefCell::new(Box::pin(task)), - scope, - waker: futures_util::task::waker(Arc::new(LocalTaskHandle { - id: task_id, - tx: self.sender.clone(), - })), + let task = Rc::new(LocalTask { + parent: self.current_task(), + task: RefCell::new(Box::pin(task)), + scope, + waker: futures_util::task::waker(Arc::new(LocalTaskHandle { + id: task_id, + tx: self.sender.clone(), + })), + }); + + entry.insert(task.clone()); + + (task, task_id) }; + // Get a borrow on the task, holding no borrows on the tasks map + debug_assert!(self.tasks.try_borrow_mut().is_ok()); + debug_assert!(task.task.try_borrow_mut().is_ok()); + let mut cx = std::task::Context::from_waker(&task.waker); if !task.task.borrow_mut().as_mut().poll(&mut cx).is_ready() { @@ -70,23 +81,18 @@ impl Runtime { .expect("Scheduler should exist"); } - entry.insert(task); - task_id } pub(crate) fn handle_task_wakeup(&self, id: Task) { - let mut tasks = self.tasks.borrow_mut(); + let task = self.tasks.borrow().get(id.0).cloned(); - let task = match tasks.get(id.0) { - Some(task) => task, - // The task was removed from the scheduler, so we can just ignore it - None => return, + // The task was removed from the scheduler, so we can just ignore it + let Some(task) = task else { + return; }; - use std::task::Context; - - let mut cx = Context::from_waker(&task.waker); + let mut cx = std::task::Context::from_waker(&task.waker); // update the scope stack self.scope_stack.borrow_mut().push(task.scope); @@ -103,7 +109,7 @@ impl Runtime { .remove(&id); // Remove it from the scheduler - tasks.try_remove(id.0); + self.tasks.borrow_mut().try_remove(id.0); } // Remove the scope from the stack @@ -120,7 +126,7 @@ impl Runtime { /// Drop the future with the given TaskId /// /// This does not abort the task, so you'll want to wrap it in an abort handle if that's important to you - pub(crate) fn remove_task(&self, id: Task) -> Option { + pub(crate) fn remove_task(&self, id: Task) -> Option> { let task = self.tasks.borrow_mut().try_remove(id.0); // Remove the task from the queued tasks so we don't poll a different task with the same id diff --git a/packages/dioxus/src/launch.rs b/packages/dioxus/src/launch.rs index a5df6cb3e..69e834d06 100644 --- a/packages/dioxus/src/launch.rs +++ b/packages/dioxus/src/launch.rs @@ -138,6 +138,7 @@ impl LaunchBuilder { .platform_config .and_then(|c| c.downcast().ok()) .unwrap_or_default(); + (self.launch_fn)(app, self.contexts, *cfg) } } @@ -145,12 +146,16 @@ impl LaunchBuilder { mod current_platform { #[cfg(all(feature = "desktop", not(feature = "fullstack")))] pub use dioxus_desktop::launch::*; + #[cfg(feature = "fullstack")] pub use dioxus_fullstack::launch::*; + #[cfg(all(feature = "web", not(any(feature = "desktop", feature = "fullstack"))))] pub use dioxus_web::launch::*; + #[cfg(not(any(feature = "desktop", feature = "web", feature = "fullstack")))] pub type Config = (); + #[cfg(not(any(feature = "desktop", feature = "web", feature = "fullstack")))] pub fn launch( root: fn() -> dioxus_core::Element, diff --git a/packages/fullstack/examples/axum-hello-world/src/main.rs b/packages/fullstack/examples/axum-hello-world/src/main.rs index 93f88beb7..f5ed24b37 100644 --- a/packages/fullstack/examples/axum-hello-world/src/main.rs +++ b/packages/fullstack/examples/axum-hello-world/src/main.rs @@ -48,6 +48,7 @@ async fn get_server_data() -> Result { fn main() { #[cfg(feature = "web")] tracing_wasm::set_as_global_default(); + #[cfg(feature = "ssr")] tracing_subscriber::fmt::init(); diff --git a/packages/fullstack/src/launch.rs b/packages/fullstack/src/launch.rs index 1a0caa5ce..08e77da6a 100644 --- a/packages/fullstack/src/launch.rs +++ b/packages/fullstack/src/launch.rs @@ -1,6 +1,5 @@ //! This module contains the `launch` function, which is the main entry point for dioxus fullstack -use core::panic; use std::any::Any; use dioxus_lib::prelude::{Element, VirtualDom};