Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't have Rc cycles in DioxusElement or Queries #3618

Merged
merged 1 commit into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/desktop/src/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ pub(crate) struct DesktopEvaluator {
impl DesktopEvaluator {
/// Creates a new evaluator for desktop-based targets.
pub fn create(desktop_ctx: DesktopContext, js: String) -> GenerationalBox<Box<dyn Evaluator>> {
let ctx = desktop_ctx.clone();
let query = desktop_ctx.query.new_query(&js, ctx);
let query = desktop_ctx.query.new_query(&js, desktop_ctx.clone());

// We create a generational box that is owned by the query slot so that when we drop the query slot, the generational box is also dropped.
let owner = UnsyncStorage::owner();
Expand Down
34 changes: 20 additions & 14 deletions packages/desktop/src/element.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,24 @@
use std::rc::Rc;

use dioxus_core::ElementId;
use dioxus_html::{
geometry::{PixelsRect, PixelsSize, PixelsVector2D},
MountedResult, RenderedElementBacking,
};

use crate::{desktop_context::DesktopContext, query::QueryEngine};
use crate::{desktop_context::DesktopContext, query::QueryEngine, WeakDesktopContext};

#[derive(Clone)]
/// A mounted element passed to onmounted events
pub struct DesktopElement {
id: ElementId,
webview: DesktopContext,
webview: WeakDesktopContext,
query: QueryEngine,
}

impl DesktopElement {
pub(crate) fn new(id: ElementId, webview: DesktopContext, query: QueryEngine) -> Self {
let webview = Rc::downgrade(&webview);
Self { id, webview, query }
}
}
Expand All @@ -28,10 +31,13 @@ macro_rules! scripted_getter {
Box<dyn futures_util::Future<Output = dioxus_html::MountedResult<$output_type>>>,
> {
let script = format!($script, id = self.id.0);

let webview = self
.webview
.upgrade()
.expect("Webview should be alive if the element is being queried");
let fut = self
.query
.new_query::<Option<$output_type>>(&script, self.webview.clone())
.new_query::<Option<$output_type>>(&script, webview)
.resolve();
Box::pin(async move {
match fut.await {
Expand Down Expand Up @@ -80,11 +86,11 @@ impl RenderedElementBacking for DesktopElement {
self.id.0,
serde_json::to_string(&behavior).expect("Failed to serialize ScrollBehavior")
);

let fut = self
.query
.new_query::<bool>(&script, self.webview.clone())
.resolve();
let webview = self
.webview
.upgrade()
.expect("Webview should be alive if the element is being queried");
let fut = self.query.new_query::<bool>(&script, webview).resolve();
Box::pin(async move {
match fut.await {
Ok(true) => Ok(()),
Expand All @@ -106,11 +112,11 @@ impl RenderedElementBacking for DesktopElement {
"return window.interpreter.setFocus({}, {});",
self.id.0, focus
);

let fut = self
.query
.new_query::<bool>(&script, self.webview.clone())
.resolve();
let webview = self
.webview
.upgrade()
.expect("Webview should be alive if the element is being queried");
let fut = self.query.new_query::<bool>(&script, webview).resolve();

Box::pin(async move {
match fut.await {
Expand Down
9 changes: 5 additions & 4 deletions packages/desktop/src/query.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::DesktopContext;
use crate::{DesktopContext, WeakDesktopContext};
use futures_util::{FutureExt, StreamExt};
use generational_box::Owner;
use serde::{de::DeserializeOwned, Deserialize};
Expand Down Expand Up @@ -107,7 +107,7 @@ impl QueryEngine {
id: request_id,
receiver: rx,
return_receiver: Some(return_rx),
desktop: context,
desktop: Rc::downgrade(&context),
phantom: std::marker::PhantomData,
}
}
Expand Down Expand Up @@ -140,7 +140,7 @@ impl QueryEngine {
}

pub(crate) struct Query<V: DeserializeOwned> {
desktop: DesktopContext,
desktop: WeakDesktopContext,
receiver: futures_channel::mpsc::UnboundedReceiver<Value>,
return_receiver: Option<futures_channel::oneshot::Receiver<Result<Value, String>>>,
pub id: usize,
Expand All @@ -161,7 +161,8 @@ impl<V: DeserializeOwned> Query<V> {
let data = message.to_string();
let script = format!(r#"window.getQuery({queue_id}).rustSend({data});"#);

self.desktop
let desktop = self.desktop.upgrade().ok_or(QueryError::Finished)?;
desktop
.webview
.evaluate_script(&script)
.map_err(|e| QueryError::Send(e.to_string()))?;
Expand Down
Loading